-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix pre commit in container #2306
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
base: main
Are you sure you want to change the base?
Fix pre commit in container #2306
Conversation
This is needed to run pre_commit in the Docker environment.
|
I'm not sure this is desired. pre-commit hooks don't require any dependency that is handled with the container setup, and I believe the standard/common practice is to keep anything git related on the host env. Can you please give more details about the usecase? |
|
The use case is running the pre-commit checks in a local development environment using Docker, which a few of us tried to do at a recent sprint. Maybe these tweaks belong in a separate build stage, though, if there are cases where they should not be applied. |
Sorry, I'm missing why. Hooks can be called separately, but the expected flow to use them is to trigger the hooks during a commit, automated via git. Making a commit in a container requires access to the hosts .git config, which is not easy or feasible to provide. |
|
I don't recall anyone committing from the container. The use case is to check that the hooks succeed independent of committing without having to first commit, go online, push, open this page, let the CI run and open the result. The container happens to be a convenient place to run the hooks since it seems to have the required dependencies, including e.g. git which doesn't work due to the safe directory issue. |
I'm not opposing the case, but just wanted to clarify, the hooks don't depend on any specific dependency in the container. Only dependency, afaik, is pre-commit. |
|
Right, after reading about pre-commit, it seems the intention of that project is that its user should not have to install any extra dependencies to get the hooks running. Is it a bug in pre-commit, then, that we have to install |
|
Aha, it looks like recent node installs now need libatomic. |
docker compose run --rm web python -m pre_commit run --all-filesdoes not currently work. Here are the changes needed to get it running properly.