config file for https://pre-commit.com/#5785
Conversation
asottile
left a comment
There was a problem hiding this comment.
Otherwise, looks good! Hope you enjoy pre-commit :)
| - id: check-merge-conflict | ||
|
|
||
| - repo: https://github.com/pescobar/pre-commit-hooks-easybuild | ||
| sha: master |
There was a problem hiding this comment.
Sorry about the drive-by review -- was just doing some googling and came across this.
Note that branch names aren't going to be repeatable and so they're discouraged
| # so the hooks are deployed to .git/hooks | ||
| # | ||
| # See http://pre-commit.com for more information | ||
| # See http://pre-commit.com/hooks.html for more hooks |
There was a problem hiding this comment.
just a note, we recently switched to https :)
@pescobar Is that also done for easyconfig files? If so, that's not a good idea, since EasyBuild injects constants and stuff like that in the environment in which the easyconfig files are parsed, so they may not parse as Python code, but that doesn't necessarily mean they're broken... |
|
@asottile thanks for the feedback! I have created a tag @boegel I have updated the config so the |
|
I think All of the currently checked in $ git ls-files -- '*.eb' | xargs check-ast
$ echo $?
0
$ git ls-files -- '*.eb' | wc -l
9493
|
check for eb files
|
@boegel I think this one is ready to merge. Please review? |
| - id: end-of-file-fixer | ||
| - id: check-merge-conflict | ||
|
|
||
| - repo: https://github.com/pescobar/pre-commit-hooks-easybuild |
There was a problem hiding this comment.
@pescobar Wouldn't it be better to host this under easybuilders?
There was a problem hiding this comment.
OK, then you should transfer ownership of that repo to the easybuilders organisation, via https://github.com/pescobar/pre-commit-hooks-easybuild/settings
There was a problem hiding this comment.
@boegel I have transferred the repo to the easybuilders organization
I have granted access permissions to all the maintainers in easybuilders. Please let me know if you think the permissions should be adjusted
| - id: check-merge-conflict | ||
|
|
||
| - repo: https://github.com/pescobar/pre-commit-hooks-easybuild | ||
| sha: v0.1 |
There was a problem hiding this comment.
It's an unfortunate mistake, this is correct though (and tags are preferred here).
When I initially made the framework it only worked with sha1 references. It's confusing, but difficult to change (the oldest open issue in our tracker is to migrate this)
There was a problem hiding this comment.
An update on this btw, the latest version of pre-commit supports rev as well as sha (originally part of v1.7.0)
|
@boegel what do you think about merging this one? |
|
@asottile right now to define a custom pep8 config I am including an extra file Is this the recommended approach or is there any way to include the custom pep8 config in thanks in advance for your help! |
|
@pescobar I usually keep them separate -- though any pep8 configuration could be set as The benefit to keeping them separate is the pre-commit hook will still read that configuration, but any other tool (IDE plugin, etc.) can also read it :) |
|
@asottile thanks for the feedback! I will keep it in a separate file as you recommend |
|
@boegel I have synced the repo with develop |
This PR includes a config file for https://pre-commit.com/ which runs all these checks for any commit:
Many of these checks are also done by Travis but travis usually takes longer to run the checks and trigger the error so this could help to reduce Travis waiting time for common mistakes and also avoid commit messages like "typo fix".."style fix"...and similars one