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

ci: add validation with yamllint #951

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

dklimpel
Copy link
Contributor

@dklimpel dklimpel commented Jul 13, 2024

Checklist
  • make test-all (UNIX) passes. CI will also test this
  • unit and/or integration tests are included (if applicable)
  • documentation is changed or added (if applicable)

Description of change

#568 adds an yamllint configuration. But it seems not to be used.

This adds a pipeline and fix a lot of issues.


📚 Documentation preview 📚: https://goss--951.org.readthedocs.build/en/951/

@dklimpel dklimpel requested a review from aelsabbahy as a code owner July 13, 2024 21:24
@aelsabbahy
Copy link
Member

Will this fail with files generated by goss. I recall a while back someone reporting goss generated yaml files don't pass yamllint.

Goss uses the go yaml package and doesn't really attempt to manipulate it.

@dklimpel
Copy link
Contributor Author

When validating Go templates, similar problems occur as in:

Goss uses the go yaml package and doesn't really attempt to manipulate it.

That is th reason why there is a list of exceptions:

ignore:
  # uses go templates (these are invalid yaml files)
  - integration-tests/goss/goss-service.yaml
  - integration-tests/goss/goss-shared.yaml
  - docs/goss.yaml

What should be next?

  • Close this PR and delete the unsused .yamllint from repo?
  • Solve the known ‘errors’ in the yaml files in a separate PR without automatic validation with the CI?
  • Leave the list of exceptions as it is and continue with this PR? Possibly divide the changes from this PR into smaller PRs
  • Or describe the list of exceptions more globally?
  • Others?

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