-
Notifications
You must be signed in to change notification settings - Fork 7k
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
add prettier as non-code auto formatter #5158
base: main
Are you sure you want to change the base?
Conversation
💊 CI failures summary and remediationsAs of commit 3f823f1 (more details on the Dr. CI page):
1 failure not recognized by patterns:
This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
Thanks @pmeier , if we add this as a pre-commit hook we would also need to add guidelines on how to run it manually. I feel like this would make our contribution process a bit heavy, especially for new contributors. It would also make our contributing guidelines a bit longer, scaring away potential contributors. Considering the small benefit (IMHO) that this check would bring, I would personally prefer not introducing the checks, and just collectively remember to limit line lengths in files. |
We only explain vision/.pre-commit-config.yaml Line 37 in bbeb320
will be enforced, but is not mentioned. Worse, vision/.pre-commit-config.yaml Lines 5 to 11 in bbeb320
are not even executable without
Can't we make
Note that this is not only line-wrapping in markdown files. This also automatically formats yaml and toml files. Previously, we only checked if there were syntactically correct, but didn't enforce an format. With this PR, 20 files would get fixed. |
We don't explain how to use the other checks used in the hooks because those very rarely fail, or are very rarely triggered by most non core-devs. Personally I don't feel like formatting yaml or even .md files is such an important priority. |
Follow-up to #5133 (review)