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

Adding YAML formatter #141

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

leahwicz
Copy link
Contributor

resolves #137

Problem

Have a linter and formatter for GitHub workflows

Solution

This is one possible solution after looking at a few possibilities here is a summary of my findings

Linter Options

Formatter Options

This PR shows the top vote options enabled and their changes.

I have it only enabled for GitHub workflow files at the moment. I also had it format the .pre-commit-config.yaml file as well since it was all over the place. If we enable it for all YAML files then a bunch of the changie files will change which I'm not sure how that would impact changie at the moment.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@cla-bot cla-bot bot added the cla:yes label Aug 12, 2024
@leahwicz
Copy link
Contributor Author

@mikealfare I listed what I found on each of the options for linters and formatters above and why I picked these 2. I didn't see too many other options that looked any different out there. Let me know if this is what you were hoping the for linter and formatter to achieve.

I know you mentioned kebab casing, capitalization, etc. standardizing as well but none of the formatters are enforcing things like that so we would have to look at some more extensive tooling if you want that kind of standard enforced.

- unlabeled
- synchronize
pull_request:
types:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why this list gets indented whereas lists in other files, e.g. pre-commit-config.yaml align the bullets with the parent node?

rev: v2.14.0
hooks:
- id: pretty-format-yaml
args: [--autofix, --indent, '2', --offset, '2', --preserve-quotes]
Copy link
Contributor

Choose a reason for hiding this comment

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

We should either make this a long form list (like black's args) or make other lists short form. I think one of the formatters also had an option for enforcing that automatically.

@mikealfare
Copy link
Contributor

@mikealfare I listed what I found on each of the options for linters and formatters above and why I picked these 2. I didn't see too many other options that looked any different out there. Let me know if this is what you were hoping the for linter and formatter to achieve.

I know you mentioned kebab casing, capitalization, etc. standardizing as well but none of the formatters are enforcing things like that so we would have to look at some more extensive tooling if you want that kind of standard enforced.

Thanks for the PR @leahwicz! I agree with your assessment above. I left two comments. Let me know your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI/CD Improvements] Standardize and automate formatting for workflows
2 participants