-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Add and use python for formatting. #6298
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
base: master
Are you sure you want to change the base?
Add and use python for formatting. #6298
Conversation
3ab309b
to
af06225
Compare
This seems to work as far as I have tested with various changes in files, both with pre-commit and here in CI calling the same python script. |
.dev/format.py
Outdated
import subprocess | ||
import argparse | ||
|
||
WHITELIST = [ |
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.
Can we put these paths into a separate file (e.g. .dev/paths_to_format.txt
) and then read that file both in format.py and format.sh? To avoid duplication
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.
I was actually thinking to remove the format.sh after a while, seeing that the python script performs as intended. So I could use the same list/file for both manual/ci formatting and for pre-commit.
We can do the split if you prefer to keep the format.sh and perhaps AI can help with the shell programming 😄
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.
I think I would prefer to keep format.sh for now. The shell command cat
is probably sufficient to print the content of the file, so maybe local whitelist="${cat ${PCL_DIR}/.dev/paths_to_format.txt}"
?
a18a512
to
329bb13
Compare
More cross platform than shell script.