Commit hooks for post commit black formatting
This PR adds a post commit git hook for automatic formatting of tracked python files in the MCDC repository.
I added the install_commit_hooks.sh script to our install.sh so the hook will be installed for new users automatically. To install the hook on an existing repo just run the install script manually bash install_commit_hooks.sh. The install script ensures black is installed, ensures a post commit file exists (if not, one is created), and adds a command to run the post_commit.git_black_format script after each new commit.
post_commit.git_black_format ensures black is installed, checks that all modified tracked files have been added or stashed, runs the black formatter on tracked python files, if there is no change in the git files after the formatting step then the script exits. If there are changes, these are amended to the most recent commit. There is also a flag to turn on/off the formatting check so the script is not called recursively.
Hopefully this structure makes sense and is a nice quality of life improvement for everyone.
We might want to consider a tools/ folder or something similar to store the bash scripts in?
Hi, Sam. Would this have a similar effect to #228?
Oh I wasn't aware of that PR, sorry. I thought last dev meeting we mentioned wanting to automate the formatting but I must have misunderstood. Yes, at first glance it looks like they achieve the same thing. I just adapted the method used in OpenMC.
Doesn't matter to me which way we go. This method avoids a dependency but also relies on bash scripts which could mean more maintenance in the future.
This method avoids a dependency but also relies on bash scripts which could mean more maintenance in the future.
That's a good assessment, Sam. Thanks! We will try with #228 for now. If, down the road, it does not work well, we can restore this PR and try the bash approach.
I should've reported the comparison tests of #228 (pre-commit dependency) VS #237 (bash-scripting).
Both correctly black-reformat at commit, but they do it differently:
- #237 (bash-scripting) reformats the files and proceed with the commit.
- #228 (pre-commit dependency) reformats the files, but it does not continue the commit. We need to
git addthe black-formatted file and then repeat the commit command.
The extra step (but minimal, only git add . and repeat the second to last command) with #228 actually makes it safer because:
(1) It gives us a chance to check how the files are actually formatted, so we are always fully aware of what we commit.
(2) It makes sure that only syntax-accurate and black-formatted code is committed. If the code is not syntax-accurate, #228 will spit out the black-formatting error and, as usual, will not proceed with the commit. However, with #237, codes that are not syntax-accurate will still get committed (the black-formatting error will still be reported, though).
There is also a hiccup in #237. In a perhaps rare situation, If the codes only need formatting, but the rest of the content is the same (such that the only changes are making the code that is already black-formatted into not black-formatted), it will still commit the not black-formatted code. In this situation, black formatting occurs after the commit, so that we need to do back-in-black commit after that.
cc: @spasmann , @clemekay