MattiSG
commented
3 years ago Thank you for this suggestion @maukoquiroga! While I agree with the importance of a consistent style and quality guidelines, I am not certain that systematic pre-commit hooks is the best way to achieve that goal.
First of all, it is not directly possible to enforce hooks from repos themselves, as they change local configuration on the host machine. At best, they could be set up automatically in the country-template (https://github.com/openfisca/country-template/issues/110), and strongly suggested in CONTRIBUTING.md and documentation, with pre-written code snippets to add as hooks.
Second, my experience is that pre-commit hooks are a strong source of frustration. Indeed, it is very common to write code that does work but does not respect all linting constraints at first. Checking style guides and refusing to commit if they are not respected makes for a frustrating experience, where it feels like the tool is working against you instead of with you. This pushes towards either deactivating the hooks for experienced users, or driving away from atomic commits for more novice users, where they feel like they have to pass every test before committing.
Considering my experience working with contributors in country packages, I would even worry that some get blocked and stop committing altogether, asking for help from more senior contributors before they can use Git at all.
An alternative could be to apply style guides instead of checking them, but my experience is that this tends to lead to problems such as: IDEs not reloading properly the files that were changed outside from the editor, with constant rewrite of content; introduction of additional syntax problems when code is committed that has an invalid syntax; blocking pre-commit hook when files can not be handled by the tool; incomprehension from users who see additional blank lines added to their files without their knowledge.
I am thus way more in favour of keeping and potentially improving the current style checks that are done in CI, letting experienced users handle their style improvements along the way if they wish to do so, and letting less technical contributors be helped by more experienced ones to pass CI checks.
I could see the value of an experiment adding a pre-push hook that would warn users that the CI checks will fail, still providing them with the interactive option to push nonetheless 🙂
As currently suggested, however, I am not in favour of deploying pre-commit hooks on contributors’ environments.
MaxGhenis
When I'm developing an extension or contributing to this repository, I want style guide to be enforced automatically and consistently across OpenFisca repositories, so I can reduce my mental overhead and add value to users instead.