search

VROOM-Project / vroom-scripts

BSD 2-Clause "Simplified" License
34 stars 20 forks source link

Format all python codes using black #25

Closed sadikkuzu closed 3 years ago

sadikkuzu commented 3 years ago

Apply black formatting to all codes via pre-commit run --all-files

jcoupey commented 3 years ago

Thanks for spotting this. I actually noticed after merging #24 that the benchmark folder was untouched by black and I applied the changes in a commit to master which I did not push right away...

Is there a straightforward way to set the --all-files flag in the pre-commit hook from .pre-commit-config.yaml? Or maybe supply the target folders?

sadikkuzu commented 3 years ago

Since pre-commit acts on the changed files during git hooks, it's a good idea to run the hooks against all of the files when adding new hooks.

While committing this https://github.com/VROOM-Project/vroom-scripts/commit/2c7d341fb2d04937c964aaccde43804a6770d462, did you use only black? or pre-commit + black? If it was second case, I guess you added some blank lines like that: https://github.com/VROOM-Project/vroom-scripts/pull/24/commits/2c7d341fb2d04937c964aaccde43804a6770d462#diff-312e71a40d64815816daae73e7ad242850265ba787cf156202d2a7db840f6f74R13 That's why pre-commit caught only those files and black formatted them.

Maybe always_run can help, but I don't know.

jcoupey commented 3 years ago

Thanks for the feedback! You're right on the one-time all-files run: I though I first did that in my initial commit where I manually applied black (without pre-commit). For some reason it only touched the files in src then...

It also means that now the move has been made, the current pre-commit setup should be fine (formatting applied to changed files).

I did not add any blank lines like the one you pointed. I noticed those after formatting and was a bit surprised but I guess this is part of the default black settings.