-
Notifications
You must be signed in to change notification settings - Fork 17
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
Adding yamlfix formatter #143
base: main
Are you sure you want to change the base?
Conversation
@mikealfare so I dug a little more after the comments you left on draft PR #141 and I think I found a better solution. You mentioned long/short list form which my original proposal did not have an option to enforce. I ended up finding |
@@ -0,0 +1,5 @@ | |||
explicit_start=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikealfare we can configure what we want here. The default currently enforces a character limit per line which is what a lot of these changes are. Before I would merge any of these, I would want to test each workflow to make sure the line break doesn't mess anything up. I would also argue that we should format quotes to all be the same but that would also require testing the workflows as well to make sure that doesn't change behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. I only have two concerns really.
First, it would be nice if yamlfix
supported command line args so that we could avoid the separate config file. It's nice having all of this config in one place in .pre-commit-config.yaml
.
Second, I need to see if we can use this tool since it's GNU GPLv3. This license combines with the Apache License, but the result needs to be licensed under GNU GPLv3. We're not actually shipping it as part of the product, but it's contributing to the quality of it. I'll consult internally to see.
@mikealfare we can't pass the configuration in as args unfortunately (I know it seems odd). The other alternative is placing them in the
Also, good call on the license. This was just an idea so we can go back to the original proposal #141 if that is better. Just let me know what makes sense to everyone. I'm happy to do either way. |
resolves #137
Problem
Have a linter and formatter for GitHub workflows
Solution
This is an alternative formatter to the one proposed here: #141