Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add YAML Linter GHA #2786

Closed
wants to merge 4 commits into from
Closed

Add YAML Linter GHA #2786

wants to merge 4 commits into from

Conversation

nefrathenrici
Copy link
Member

@nefrathenrici nefrathenrici commented Mar 13, 2024

This PR adds a basic YAML linter to keep our config files clean.

Two open questions

  • Should this action lint all YAML files, including pipeline.yml?
  • Should this action be part of the existing Julia formatter action?

If people like this, I will format the YAML files nicely in this PR as well.

@nefrathenrici nefrathenrici marked this pull request as ready for review March 13, 2024 21:17
@nefrathenrici nefrathenrici changed the title Test yaml linter Add YAML Linter GHA Mar 13, 2024
Copy link
Member

@charleskawczynski charleskawczynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, maybe someone else can review this, too?

@Sbozzolo
Copy link
Member

Can you post what the diff/output would look like?

@nefrathenrici
Copy link
Member Author

Here is some sample output - it is not as nice as the Julia formatter because it just lints instead of formatting.

I am looking into yamlfmt for actual formatting

@nefrathenrici nefrathenrici force-pushed the ne/yaml_lint branch 3 times, most recently from 1ccd8ee to 151432e Compare March 19, 2024 23:37
@nefrathenrici
Copy link
Member Author

nefrathenrici commented Mar 19, 2024

@Sbozzolo I have gotten it to run the diff, here is some sample output below. The workflow log contains color for the diff as well.

--- a/config/gpu_configs/gpu_aquaplanet_diagedmf.yml
+++ b/config/gpu_configs/gpu_aquaplanet_diagedmf.yml
@@ -7,23 +7,23 @@ z_max: 55000.0
 z_elem: 63
 dz_bottom: 30.0
 dz_top: 3000.0
-moist: equil 
-surface_setup: DefaultMoninObukhov 
+moist: equil
+surface_setup: DefaultMoninObukhov
 rad: allskywithclear
 idealized_insolation: false
 dt_rad: 1hours
 dt_cloud_fraction: 1hours
-turbconv: diagnostic_edmfx 
+turbconv: diagnostic_edmfx
 implicit_diffusion: true

@Sbozzolo
Copy link
Member

If 90 % of what this code does is removing trailing spaces, I think we should set up something that removes trailing spaces from every file. We have tons of them.

@nefrathenrici nefrathenrici force-pushed the ne/yaml_lint branch 3 times, most recently from b6ccf63 to 7f7773e Compare March 20, 2024 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants