Skip to content

Conversation

@ahuber21
Copy link
Contributor

@ahuber21 ahuber21 commented Dec 10, 2025

I noticed inconsistent configuration of yml autoformatters, so I added a formatter to the pre-commit hooks.

mihaic
mihaic previously requested changes Dec 10, 2025
Copy link
Member

@mihaic mihaic left a comment

Choose a reason for hiding this comment

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

Great idea! Can we use another repo? The pre-commit mirror repo has been archived.

@ahuber21
Copy link
Contributor Author

How about using ymlfmt instead, which is based on ruamel.yaml
https://yaml.dev/doc/ruamel.yaml/

@ahuber21 ahuber21 changed the title Add prettier as yml linter Add yml linter Dec 11, 2025
@mihaic
Copy link
Member

mihaic commented Dec 11, 2025

How about using ymlfmt instead, which is based on ruamel.yaml https://yaml.dev/doc/ruamel.yaml/

It looks like it is not keeping up with ruamel.yaml development:

    'ruamel.yaml >=0.16.10, <=0.17.21',

https://github.com/jumanjihouse/pre-commit-hook-yamlfmt/blob/master/setup.py#L12
https://pypi.org/project/ruamel.yaml/#history

I am not convinced by ymlfmt either but I will remove my change request in case someone else wants to approve.

@mihaic mihaic dismissed their stale review December 11, 2025 19:28

No longer using the hook I objected to.

Copy link
Member

@ethanglaser ethanglaser left a comment

Choose a reason for hiding this comment

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

No objections from my end. Seems like an industry standard tool with fairly typical args. Thanks for unifying

@ahuber21
Copy link
Contributor Author

Okay, this time I went with check-yaml from pre-commit-hooks directly. The downside is that it's not able to format files that don't meet requirements. But it will still flag them of course.
Since I'm not able to find a well-maintained formatter hook, I suggest we go with linting only.

All yaml files still pass the check. I tested with

pre-commit run --all-files

@mihaic mihaic merged commit 640ee6d into main Dec 12, 2025
15 checks passed
@mihaic mihaic deleted the dev/add-yml-linter branch December 12, 2025 16:59
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.

4 participants