-
Notifications
You must be signed in to change notification settings - Fork 445
Re-adopt pre-commit hook with Black reformatter (or not?) #709
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
Re-adopt pre-commit hook with Black reformatter (or not?) #709
Conversation
`24.8.0` is the last version that supports python 3.8. The stable version of black currently supports versions newer than python 3.9 only. Also, adding paths after the hook entry is meaningless because pre-commit will append all staged files behind the black command automatically. The version of python black expects is bumped to 3.8. Signed-off-by: dodo920306 <[email protected]>
According to `docs/index.md`, Cello uses `black` code style. However, the code here doesn't really complies with that. This commit reformats all the Python codes here by `black`. Signed-off-by: dodo920306 <[email protected]>
|
Please consider and answer the question before merging this. Also, another problem is should this be integrated with the CI pipeline? |
|
I think using tox with flake8 in the CI pipeline is enough? @YoungHypo what is your favor? |
Signed-off-by: Yu-Lin "Kirin" Chu <[email protected]>
If we decide to adopt black for code formatting, adding a black check step in the CI could help us catch unformatted code early, before it gets merged. @yeasy @dodo920306 |
I have no opinion. If we want to do it, we have to make a new pipeline step to execute |
Signed-off-by: Yu-Lin "Kirin" Chu <[email protected]>
Signed-off-by: dodo920306 <[email protected]>
Signed-off-by: Yu-Lin "Kirin" Chu <[email protected]>
|
after discussion, we will take:
|
Signed-off-by: Yu-Lin "Kirin" Chu <[email protected]>
According to
docs/index.md, Cello usesBlackcode style.Moreover, there is a
.pre-commit-config.yamlfile about this.However, the current code here doesn't seem to be like that.
This PR is about whether to keep
Blackusage as recorded, or is simply usingtoxwithflake8in the CI pipeline enough?To use it, you must run
so that the hook can be applied every time you commit. (Check out https://github.com/pre-commit/pre-commit for more info.)
Optionally, you can run
to trigger this without commit.