-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Pre Commit
To automatically ensure that certain formatting practices are maintained throughout the codebase, and that any incoming pull requests pass a set of requisite JavaScript and Python checks, Open Library uses GitHub's Continuous Integration (CI) Server with a set of pre-commit hooks to run a series of automated checks on incoming PRs.
If you don't have pre-commit installed locally and your PR fails any one of the checks, there are two things that may happen when you push your changes:
- One of the checks initially fails, but then the
pre-commitbot pushes a new commit for you that fixes the problem. If this is the case, you'll see a commit that looks like this:
This means that the pre-commit bot auto-fixed the problem for you, which is very common in the case of simple formatting errors. In this case, you're all set, but if you plan on making any more changes to your branch, it's a good idea to pull in pre-commit's changes with git pull origin HEAD to ensure you don't run into any conflicts.
- The check simply fails and you'll see something that looks like this:
This means that the problem the CI identified requires human intervention. You'll want to click "Details" to see what caused the test to fail, try to fix the problem locally, and push up a new solution with git push origin HEAD. If you're not sure what is causing the error or how to fix it, this is a great time to reach out to the issue's lead for guidance on how to proceed.
To test everything the CI server checks (JavaScript, mypy, black, ruff, etc.), and to do so automatically at the time of commit, one can run, in the local environment, outside of Docker, a Python program named pre-commit. This will use git's hooks to run Open Library-specific linting checks when committing code in git. Because pre-commit integrates with git, that means it runs outside of Docker, and needs to be available to git in your current environment.
If you have pre-commit installed, the checks will run locally each time you add a commit to your branch. This way, you don't have to worry about re-pushing your changes every time you encounter a pre-commit error, which is especially nice in the case of simple formatting issues like trailing whitespace.
If your commit fails any of the checks, there are two things that may happen:
- The check will initially fail and your staged changes will not be committed, but
pre-commitwill auto-add a new change that fixes the problem, in which case you'll see a new unstaged git change and something like this:
All you need to do in this case is add the change to staging and commit again. The check should pass, and you should now be able to push your changes.
- The check will simply fail, your staged changes will not be committed, and you'll see an error message like this:
This means that the problem requires human intervention, which means that you can fix the problem locally, using the error message info and/or guidance from the issue's lead, and then re-commit and push your changes as needed.
Prerequisites:
- the version of your current Python interpreter must match the version of Python specified in the
default_language_versionsection of.pre-commit-config.yaml.
Although a complete discussion of managing Python's versions and Python's virtual environments is outside the scope of this discussion, it is likely worth creating a virtual environment for each Python project on which you work. See Python's own documentation about venv for one such approach to managing virtual environments. Additionally, if your python3 --version doesn't match the version specified in .pre-commit-config.yaml, consider pyenv on Linux, macOS, or Windows Subsystem for Linux, or pyenv-win on Windows outside of the Windows Subsystem for Linux.
Note: this will install a git commit hook that will run prior to every commit. As there are times where one may simply wish to commit code, even if it will fail the linting, one can override commit hooks with git commit --no-verify. For more on pre-commit, see https://pre-commit.com/.
To enable pre-commit, run the following in your local shell outside of Docker:
-
pip install pre-commitorbrew install pre-commit; and pre-commit install
Henceforth, pre-commit will lint your code with every git commit (unless you commit with git commit --no-verify to disable running the hooks). To manually run pre-commit, you can execute pre-commit run --all-files.
If you see an error similar to either of the following, please ensure you the version of you Python interpreter matches the version specified in .pre-commit-config.yaml:
An unexpected error has occurred: CalledProcessError: command: ('/home/scott/.pyenv/versions/3.9/bin/python3.9', '-mvirtualenv', '/home/scott/.cache/pre-commit/repolh5wc3hy/py_env-python3.11', '-p', 'python3.11')
return code: 1
stdout:
RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.11'
stderr: (none)
Check the log at /home/scott/.cache/pre-commit/pre-commit.logTo remove pre-commit, run pre-commit uninstall.
If your commit involves adding, removing or altering text that will be visible to the user and is properly internationalized, an update of the translation template file will be automatically bundled in with your changes via pre-commit.
If you're running pre-commit locally:
- Your code will "fail" a test called
Generate POT, give you the error messageFiles were modified by this hook, and addmessages.potchanges to your git unstaged changes. - All you need to do to "pass" the test is add the
messages.potfile to staging and redo your commit; theGenerate POTtest should now pass, and your changes will be immediately available to translators once your branch is merged.
If you're not running pre-commit locally:
- Your code will "fail" the
pre-commitcheck run by the GitHub Continuous Integration (CI) server - The CI will then push a new commit to your remote branch that contains the necessary
messages.potupdates and now passes thepre-commitcheck - You don't need to do anything else after this, but if you want to make and push further changes to the PR, it would be wise to first run a
git pull origin HEADto pull in the newmessages.potchanges and avoid conflicts in future pushes