Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git hooks with pre-commit #2599

Merged
merged 12 commits into from Aug 12, 2019

Conversation

@bilke
Copy link
Member

commented Aug 7, 2019

Adds git hooks via pre-commit hooks manager.

Has to be manually enabled:

pip install pre-commit
cd ogs
pre-commit install

Currently checks for:

  • Trailing whitespace (and auto-corrects them)
  • Git LFS files to be correctly commited
  • XML syntax check
  • Yaml syntax check
  • Large (non-lfs files), rejects files larger than 1 MB
  • All file extensions have to be lower-case

Excluded directories: ThirdParty, Tests/Data

You can run it also manually on the currently staged files:

pre-commit run

Or on all files in the repo (this is also done in Jenkins):

$ pre-commit run --all-files
Trim Trailing Whitespace.................................................Passed
Check for added large files..............................................Passed
Check for merge conflicts................................................Passed
Check Xml................................................................Passed
Check Yaml...............................................................Passed
Run git diff --check.....................................................Passed
Check file extensions....................................................Passed
Check git lfs files......................................................Passed

Fixes #2592.

TODO:

  1. Feature description was added to the changelog
  2. Any new feature or behavior change was documented?
  3. Add hook to check for lower-case file extensions
  4. Does this work Windows?

@bilke bilke added the WIP 🏗 label Aug 7, 2019

@bilke bilke force-pushed the bilke:hooks branch from 10a1ad0 to ae90728 Aug 7, 2019

@bilke bilke force-pushed the bilke:hooks branch from ae90728 to 8cf7c6c Aug 9, 2019

@bilke bilke force-pushed the bilke:hooks branch from 8cf7c6c to 54ae48f Aug 9, 2019

@bilke bilke added please review and removed WIP 🏗 labels Aug 9, 2019

@bilke bilke requested a review from endJunction Aug 9, 2019

RUN pip3 install virtualenv
# CMake version 3.13.4
RUN pip3 install virtualenv pre-commit
# CMake version 3.15.1

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 9, 2019

Member

Is 3.15.1 the new minimum for cmake or only in docker?

This comment has been minimized.

Copy link
@bilke

bilke Aug 12, 2019

Author Member

no, my fault. I now set this to 3.12.4.

@endJunction
Copy link
Member

left a comment

Works with AUR's python-pre-commit package for me.

Is it possible to integrate the clang-format pre-commit hook too? See https://github.com/ufz/ogs-utils/tree/master/dev/code-formatting/clang-format-pre-commit-hook

@bilke bilke force-pushed the bilke:hooks branch from f5d5003 to d75eba8 Aug 12, 2019

@bilke

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

I have not added the linked clang-format hook as it specifially works on the acutal git diff. The pre-commit tool works a bit differently: it gives the hooks scripts the filenames of the changed files or all files in the repo (when running with --all-files). Also clang-format does not return a value if it actually formatted the file. For the moment I am not sure how to properly integrate both together...

[hooks] Added file name check.
Fixed own hooks to use the correct files passes by pre-commit.

@bilke bilke force-pushed the bilke:hooks branch from d75eba8 to f95ce87 Aug 12, 2019

@endJunction

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

@bilke bilke merged commit 8a7b703 into ufz:master Aug 12, 2019

3 checks passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details
ufz.ogs #20190812.5 succeeded
Details

@bilke bilke deleted the bilke:hooks branch Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.