Add optional .pre-commit-config.yaml for the OpenFermion project#1289
Add optional .pre-commit-config.yaml for the OpenFermion project#1289
.pre-commit-config.yaml for the OpenFermion project#1289Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a .pre-commit-config.yaml file to the OpenFermion project, configuring a comprehensive suite of hooks for linting, formatting, and security checks. The review feedback identifies several necessary corrections: fixing a regex syntax error in a local hook's file matching pattern, correcting a version mismatch in the isort hook comment, replacing a non-portable absolute path for the copyright notice template with a relative one, and removing irrelevant, non-existent paths from the exclusion list.
pavoljuhas
left a comment
pavoljuhas
left a comment
There was a problem hiding this comment.
I don't have experience with the pre-commit tool. It may be worthwhile to test this for a stock Ubuntu account without any of your local customizations. I mean, if users set up their development environment for OpenFermion and activate the pre-commit hooks, will they work out of the box?
podman run --rm -ti ubuntu:noble bash
Otherwise LGTM.
I suspect it may modify files.
It's not clear if it can modify files, but let's not risk it.
It duplicates other checks but doesn't use the same config files.
This is for use in `.pre-commit-config.yaml`, although it may be useful for other purposes too.
mhucka
added
the
area/devops
We currently don't use isort in this project.
The problem is that it flags typos in parts of a file that you didn't touch. If you fix those, you pollute your PR with unrelated changes.
mhucka
changed the title
.pre-commit-config.yaml for the OpenFermion project.pre-commit-config.yaml for the OpenFermion project
Don't need to quote the strings, plus use array notation for all of them
mhucka
changed the title
.pre-commit-config.yaml for the OpenFermion project.pre-commit-config.yaml for the OpenFermion project
- id: check-github-issue-config - id: check-github-issue-forms
Rarely relevant because the config file changes so infrequently, plus it's extremely unlikely that a non-core contributor would edit those files.
I'm not even sure what the threshold is.
We have other tools on GitHub that look for those things.
This causes more problems than it's worth.
A good question. It's a bit difficult to test doing an actual git commit, but I did the following inside a docker environment: python3 -m venv venv
source venv/bin/activate
pip install pre-commit
pre-commit install -t pre-commit -t commit-msg -t pre-push
pre-commit runAnd this works. So, I think it's reasonable to believe it will work for other people. |
This doesn't seem to appear in the code base anyway. Let's save a bit of time time by skipping it.
With a little help from my friend, Gemini.
2 participants
This adds a
.pre-commit-config.yamlbased on what I have been using for some time. Nothing invokes the pre-commit hooks directly, and it is up to the user to configure it and use it if they wish. The intention is to provide a modest pre-commit configuration that can help people avoid some common mistakes.pre-commit is a framework for managing and maintaining git hooks. It's an open-source project and has extensive documentation. It's a way for devs to run checks on their code locally before they commit or push it.