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

chore(ci): add yaml linter workflow and fix yaml formatting #98

Merged
merged 8 commits into from
Jan 12, 2025

Conversation

qdm12
Copy link
Collaborator

@qdm12 qdm12 commented Jan 2, 2025

Why this should be merged

To have consistent, warning-free yaml files, enforced by the CI

How this works

  • New CI job triggering only on yaml/yml file modifications using the built-in yamllint with a .github/yamllint.yml configuration
  • Fix existing yml files to pass CI
  • Ignore geth original files such as .travis.yml files

How this was tested

New CI job passing

@qdm12 qdm12 force-pushed the qdm12/ci/yml-linter branch from df05a9b to e41a3e3 Compare January 8, 2025 16:33
@qdm12 qdm12 marked this pull request as ready for review January 10, 2025 10:47
@qdm12 qdm12 requested a review from ARR4N January 10, 2025 10:48
paths:
- "**/*.yml"
- "**/*.yaml"
- ".github/workflows/yml.yml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of interest, why are these two explicitly required?

Copy link
Collaborator Author

@qdm12 qdm12 Jan 12, 2025

Choose a reason for hiding this comment

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

Just so the (non required) workflow only triggers when modifying .yml/.yaml files. I suppose the workflow is yml too, so no need to specify it, but I really added it out of habit without thinking this through 😄 Although thinking about it, it might be worth leaving it in case someone copies the workflow (i.e. to make a markdown one) so he/she doesn't forget to have a trigger file path on the workflow file itself.

.github/workflows/yml.yml Outdated Show resolved Hide resolved
.github/yamllint.yml Outdated Show resolved Hide resolved
.golangci.yml Outdated
template-path: .libevm-header

gomodguard:
blocked:
modules:
- github.com/ethereum/go-ethereum:
reason: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting! How did it know that these fields exist?! I've made suggestions with reasons so my original rationale is now explicit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It didn't, I looked it up because the linter was complaining about the empty object, i.e.

- blabla:
- abc:

is not making the linter happy, so I've set just one field to its zero value 🤷 But good you've added a reason for each, even better 👍

.golangci.yml Outdated Show resolved Hide resolved
.golangci.yml Outdated Show resolved Hide resolved
.golangci.yml Outdated Show resolved Hide resolved
.golangci.yml Outdated Show resolved Hide resolved
qdm12 and others added 7 commits January 12, 2025 14:43
- triggers only on yaml/yml file modifications
- uses built-in yamllint with yamllint.yml configuration
Co-authored-by: Arran Schlosberg <[email protected]>
Signed-off-by: Quentin McGaw <[email protected]>
Co-authored-by: Arran Schlosberg <[email protected]>
Signed-off-by: Quentin McGaw <[email protected]>
Co-authored-by: Arran Schlosberg <[email protected]>
Signed-off-by: Quentin McGaw <[email protected]>
Co-authored-by: Arran Schlosberg <[email protected]>
Signed-off-by: Quentin McGaw <[email protected]>
Co-authored-by: Arran Schlosberg <[email protected]>
Signed-off-by: Quentin McGaw <[email protected]>
@qdm12 qdm12 force-pushed the qdm12/ci/yml-linter branch 2 times, most recently from 66acc12 to d9d59a1 Compare January 12, 2025 13:47
@qdm12 qdm12 force-pushed the qdm12/ci/yml-linter branch from d9d59a1 to a4b5f69 Compare January 12, 2025 13:49
@qdm12 qdm12 enabled auto-merge (squash) January 12, 2025 13:53
@qdm12 qdm12 merged commit c996175 into main Jan 12, 2025
5 checks passed
@qdm12 qdm12 deleted the qdm12/ci/yml-linter branch January 12, 2025 13:56
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.

2 participants