search

BlueBrain / hpc-coding-conventions

Apache License 2.0
8 stars 5 forks source link

Nudge contributors to check their change before submitting it #81

Closed tristan0x closed 3 years ago

tristan0x commented 3 years ago

Significant computing resources and time are wasted during the continuous integration process when building and testing code that is not properly formatted. Despite the CI workflow can be designed to act cleverly whenever a contribution is not properly formatted, it is better for both developers and CI if the change is correct in the first place.

This issue wants to introduce a new CMake option named ${PROJECT}_ENABLE_POST_COMMIT_MSG, enabled by default, that will write a gentle reminder to the console with a checklist to follow before submitting a change after a git commit.

if ${PROJECT_SOURCE_DIR}/.post-commit-msg file is present, then the content of the file will be written to the console instead of the default message.

cattabiani commented 3 years ago

Nice! +1

matz-e commented 3 years ago

Maybe on every commit is a bit much (for those of us that like to amend frequently). I noticed there's also a pre-push hook that could actually check the formatting?

alkino commented 3 years ago

The problem of the message each time, it that you will stop reading it quite fast.

cattabiani commented 3 years ago

if you already know the message you press enter twice without even glancing over it. I guess this change is for newcommers?

matz-e commented 3 years ago

It also pollutes your terminal scrollback a bit? An optional pre-push message I wouldn't mind that much. clang-format by itself should be fast enough to provide a quick-pre-push check, too.

tristan0x commented 3 years ago

This message is for newcomers and external collaborators. As mentioned in the description above, the message can be easily discarded with a CMake option.

I am not against moving the message to the pre-push git event.

This project also provides pre-commit hooks that could also be moved to the pre-push event.

ohm314 commented 3 years ago

Maybe pre-push is not a bad compromise? let's put it there.. i think this is the best place. You don't do this often ,but when you create a PR that's what you'll do just before (usually). Having the reminder there would be not too verbose and it would be at a relevant time in the workflow of a developer.