Feat/pre commit checks

• Added pre-commit hooks which:
  • isort on .py and .ipynb
  • flake8 on .py and .ipynb
  • nbstripout
  • rstcheck
  • check-added-large-files
  • check-ast
  • check-json
  • check-yaml
  • check-toml
  • end-of-file-fixer
  • trailing-whitespace
  • check-docstring-first
  • check-merge-conflict
  • mixed-line-ending
• Hooks run as part of the check stage in the CI
• Fixed some of the issues:
  • EoF
  • Trailing whitespace
  • isort (but only for .py)

I included a lot of other changes and fixes in the original MR (https://git.xfel.eu/gitlab/detectors/pycalibration/merge_requests/411) before figuring out how to get the checks to run only on changed files, which meant that the MR implementing pre-commit checks also refactored a decent chunk of code. It would be better to do that refactoring in separate MRs so I've closed the original one and opened this to replace it.

@ahmedk @danilevc @hammerd feel free to review when you have time

TODO:

• Add pre-commit to requirements.txt
• Don't mutate the code during pre-commit checks - changed my mind about this a few times, left it to mutate the code as that is only done on staged files, so any mutations can be reverted by undoing changes anyway. Added an explanation on this to the readme instead.
• Add info to the readme and to the wiki
• Add explanation of what each of the checks is for in more detail
• Add scripts to execute w/ diff-only (same as the CI pipeline)

added Waiting for review label

added 7 commits

• 5f7171e5...4a3f5551 - 2 commits from branch master
• 0dd8fac3 - Update CI to work with pre-commit
• 12d8b0ec - Add pre-commit configuration
• de4b6414 - Check if MR commit SHA is set
• 7c45d20c - Use consistent isort version
• 9453cac7 - Add comments to clarify non-standard sections

Compare with previous version

changed the description

changed the description

added 1 commit

• 62a18e9b - blabla

Compare with previous version

I'd be keen to see some news on this

marked the checklist item Add pre-commit to requirements.txt as completed

added 3 commits

• 232408d9 - Add pre-commit to requirements
• e431d984 - Sort requirements
• f0b9099d - Update README.rst

Compare with previous version

added 1 commit

• 760007ca - Add todos, adjust headings

Compare with previous version

added 51 commits

• 760007ca...233b8f7e - 41 commits from branch master
• c5b8d367 - Update CI to work with pre-commit
• aceded4b - Add pre-commit configuration
• c4d550a2 - Check if MR commit SHA is set
• cae49ef4 - Use consistent isort version
• 6179aaa5 - Add comments to clarify non-standard sections
• 797af5db - blabla
• 43122cd3 - Add pre-commit to requirements
• db768d61 - Sort requirements
• d6b83d8b - Update README.rst
• b98fed75 - Add todos, adjust headings

Compare with previous version

marked the checklist item Add scripts to execute w/ diff-only (same as the CI pipeline) as completed

marked the checklist item Add info to the readme and to the wiki as completed

marked the checklist item Add explanation of what each of the checks is for in more detail as completed

added 4 commits

• 95183eb5 - Add a script for easy execution of diff-only pre-commit checks
• a142df25 - Swap lines to make more sense
• 6342f2e6 - Export SHA var as it's used during pre-commit itself
• 9ecbc83e - Add additional details to the readme

Compare with previous version

added 1 commit

• 2854f22b - Add accidentally removed dependencies

Compare with previous version

So far I left an intentional mistake in where the order of imports in a file was wrong so that the CI check would fail for them. Previous pipeline is this one where isort fails: https://git.xfel.eu/gitlab/detectors/pycalibration/-/jobs/170165

I'll fix that in the next commit, and in theory the next pipeline run should succeed

added 1 commit

• a6f43240 - Fix import order

Compare with previous version

Sweet, checks pass as expected: https://git.xfel.eu/gitlab/detectors/pycalibration/-/jobs/170168

Now I'll mark the checks as allow failures so that you can choose to ignore them during the CI stage and so that the subsequent stages run even if the checks fail, undo the import order fix to see if the CI stage is correctly marked with a warning, and then if that is the case this'll be ready for review.

added 1 commit

• d7ca8e96 - Add notes on skipping checks, set to allow failure true, break import order again

Compare with previous version

changed the description

marked the checklist item Don't mutate the code during pre-commit checks - changed my mind about this a few times, left it to mutate the code as that is only done on staged files, so any mutations can be reverted by undoing changes anyway. Added an explanation on this to the readme instead. as completed

Sweet, works as expected, isort failed and checks marked with warning: https://git.xfel.eu/gitlab/detectors/pycalibration/-/jobs/170170

Then tests continued: https://git.xfel.eu/gitlab/detectors/pycalibration/-/jobs/170171

added 1 commit

• 9e6030a3 - Fix import order

Compare with previous version

@danilevc @ahmedk @hammerd

Done everything on my list and checked that it all works as I expected (see above comments where I'm talking to myself). Ready for review now

added 1 commit

• 654c663b - Add .git-blame-ignore-revs file preemptively

Compare with previous version

added 1 commit

• 0d902b2b - Add .git-blame-ignore-revs file preemptively

Compare with previous version

mentioned in merge request !433 (merged)

mentioned in merge request !434 (merged)

LGTM, thanks a lot!

Sweet, thanks!

Very very nice work @roscar. LGTM!

Is there a way to maybe put the path of the wiki page for the guideline and codestyle?

Hmmm that was going to be one of my questions for you, discussed it a bit with Cyril and I think we should try and unify the content of the readme and the wiki pages, but that could come a bit later. If we want to potentially open source this in the future then it would probably be better to have everything in the repository (readme, docs, etc...) and move away from the wiki, but that discussion is a long way away.

For now your idea is good, I'll just add a link to the wiki under the contributing section, thanks!

added 1 commit

• b132f8c0 - Add link to wiki dev guidelines to readme

Compare with previous version

merged

mentioned in commit 9ab7db36

mentioned in commit 63df5b2f

changed the description

removed Waiting for review label