Initial config for pre-commit #2670
Conversation
- Add an initial configuration for pre-commit.com checks to run on this repository - Related to geopm#2530
| - repo: https://github.com/pre-commit/pre-commit-hooks | ||
| rev: v3.2.0 | ||
| hooks: | ||
| - id: trailing-whitespace |
There was a problem hiding this comment.
Auto-removes trailing whitespace
There was a problem hiding this comment.
Note: pre-commit has a way to tell per-file hooks to skip file patterns/types. Skip patch files for this one. e.g., by exclude_types diff or by exclude \.patch$
| rev: v3.2.0 | ||
| hooks: | ||
| - id: trailing-whitespace | ||
| - id: end-of-file-fixer |
There was a problem hiding this comment.
ensures that a file is either empty, or ends with one newline.
There was a problem hiding this comment.
also ignore patch files
| hooks: | ||
| - id: trailing-whitespace | ||
| - id: end-of-file-fixer | ||
| - id: check-yaml |
There was a problem hiding this comment.
checks yaml files for parseable syntax.
| - id: trailing-whitespace | ||
| - id: end-of-file-fixer | ||
| - id: check-yaml | ||
| - id: check-json |
There was a problem hiding this comment.
checks json files for parseable syntax.
| - id: end-of-file-fixer | ||
| - id: check-yaml | ||
| - id: check-json | ||
| - id: check-added-large-files |
There was a problem hiding this comment.
prevents giant files from being committed.
| - id: flake8 | ||
| args: ["--max-line-length=120", "--extend-ignore=E261"] |
There was a problem hiding this comment.
This will need more discussion. Our contributor guidelines say we follow pep8, but I think we're following a modified version of it. Which errors do we want to track?
| - id: bandit | ||
| # Only gate high-severity issues | ||
| args: ["-lll"] |
There was a problem hiding this comment.
This checks for common security issues in python code.
It can be pretty nit-picky by default. Should we just gate on high-severity issues?
Also note that the pre-commit framework is opt-in, so this config file doesn't actually prevent these issues from getting committed. It's a convenience for committers to see issues when they introduce them instead of later during review or during our pre-release checklist.
| - id: codespell | ||
| args: ["--ignore-words-list", "atleast,clos"] |
There was a problem hiding this comment.
This is a low-false-positive-rate code-friendly spell checker. It isn't grammar-aware or completely spelling-aware. But it does a good job of catching common mis-spellings and recommending reasonable fixes in the error message.
It also allows us to add custom confusion lists. Would we want to do that with anything? E.g. geomp -> geopm.
| - id: geopm-license | ||
| name: GEOPM License Checks | ||
| description: This hook checks whether files have the right license header. | ||
| pass_filenames: false | ||
| entry: ./check-precommit-license | ||
| language: script | ||
| types: [file] |
There was a problem hiding this comment.
We can also run scripts to do any project-specific checks. We want these to be fast things (like checking for license headers), not long things that would disrupt commit workflows (like build-and-UT). Anything else to add?
| @@ -0,0 +1,53 @@ | |||
| # See https://pre-commit.com for more information | |||
| # See https://pre-commit.com/hooks.html for more hooks | |||
There was a problem hiding this comment.
Any others from there that we want to add?
cmcantalupo
force-pushed
the
dev
branch
2 times, most recently
from
2cd441d
to
0859ce3
Compare
Some of these pre-commit tests fail on the current contents of the repo. Some pre-work exists to resolve those failures.
To run a single check on all files in the repo, use the
--all-filesoption inpre-commit run. E.g., for the geopm license checks (that one succeeds for all files right now):Or to find issues with rst file backtick usage: