Introduce pre-commit

[yngve793]

By using pre-commit it is easy to install hooks for checking various properties of the code.
Current configuration checks end of line whitespaces and end of file new line.
This is also part of the tool chain designed to help to keep up quality in further development.

[achaikou]

Very nice idea with pre-commit hooks! 🥳 Didn't even occur to me we could have something like that!

I however have questions about how you see it being used.

1. Should we make all the files compliant first?
   From what I understand every hook would run on whole file if it has commited changes.

2. Should using these pre-commit hooks be a team policy or individual policy?
   If it is a team policy, installing and using pre-commit should be somehow required of us.
   If it is an individual policy, does .pre-commit-config.yaml file has to be in the repo?

3. How slow is this?
   I've noticed that it can take significant amount of time to run current hooks (though maybe it is only slow when files are not compliant). I am used to ammend commits quite often, and delays there would be annoying 😾

[yngve793]

Yes, it is a very nice tool.

By using a shared style files and a common pre-commit setup we could remove most of the style based comments and fixups. It is probably not realistic to define style files that are compatible with the historical style being used in the code base.

Yes, the hole file is check if there is a change.

Pre-commits has to be a team policy. Otherwise there will be an ongoing battle. Keeping it in the repo ensure that everybody is up to date.

It really depends on the hooks being used. It is not necessary to run it on every commit. I usually just run in on the final rebase.

[achaikou]

It is not necessary to run it on every commit. I usually just run in on the final rebase.

Hm...
So you say you didn't do step 3 in the tutorial so your hook is not running by default?
Yes, I like your approach of running these things manually only as a final polish more! 😃

Pre-commits has to be a team policy. Otherwise there will be an ongoing battle.

OK. Does pre-commit itself has to be a team policy or just effects of pre-commit documented in .pre-commit-config.yaml?

• In first case we would need to force using the tool, which I am not fond of.
• In second - run it or don't, your choice, but if you have missing end line at the end of the file, Big Bad Wolf Reviewer will complain.
  Or CI.

Should we set it up?

• We can use https://pre-commit.ci/ : external tool will make fixing commits. I am very much not fond of this as it has potential for polluting the repo.

• Or we can create a pre-commit job in separate style.yaml workflow which will automatically fail if something is not up to standard.

    - name: Run hooks
      run: |
        pip install pre-commit
        pre-commit run --all-files

I like it more as it indirectly enforces pre-commit as a contributing policy.

[yngve793]

Step 3 is a personal choice. I only use it on oneseismic-api.

Setting it up has real value if we had a standard to follow on the c and potentially the c++ part. I find this unrealistic to achieve. It isn't uncommon to run it on push to ensure that PR are free for trivial issues.

[achaikou]

Btw fix the message for Add pre-commit hooks on rebase, it is outdated