Open TimidRobot opened 2 years ago
I see this has the tag for "Hacktoberfest". I am an aspiring intern from Outreachy. Can I use this issue to better understand the code base so I can eventually make contributions on the "Licenses Machine Readable Information" project.
I see this has the tag for "Hacktoberfest". I am an aspiring intern from Outreachy. Can I use this issue to better understand the code base so I can eventually make contributions on the "Licenses Machine Readable Information" project.
@Th3nn3ss Yep! the "Hacktoberfest" tag is not meant to limit involvement.
@TimidRobot I'm not sure I understand the issue I was able to use pre commit fine within the docker container. What behaviour are you aiming for exactly? I was also able to create a small service using the app service as a template and tried something like:
docker-compose run pre-commit git commit -a -m "some changes"
And it invoked pre-commit fine as well.
@Alig1493 Yes, you can manually invoke it on the docker container, but the primary purpose of pre-commit is to run prior to each commit (as a git hook). Also, you can run it on the existing app service (ex. docker compose exec app pre-commit run --show-diff-on-failure --color=always --all-files
).
While it may be easy enough to add a docker command as a git pre-commit hook, I expect a decent developer experience will require a helper script so that appropriate error messages are issued if a commit is tried and the docker services are unavailable. Ideally, it would be nice to find someone who has written about their experience using docker with git hooks. I'm not convinced a good developer experience is possible.
I want to contribute to this if it is still open
assign me
Please see Contribution Guidelines — Creative Commons Open Source for how we manage issues and PRs (we generally don't assign issues prior to resolution).
@TimidRobot , I have curated a plan of action for the issue. Can you please review it and let me know if I have understood the problem correctly and if any further improvements in the plan of action further. Once if the proposal is appealing I will go forward with the implementation. Here is the link of proposal : https://tan-shampoo-b5f.notion.site/Proposal-for-Using-pre-commit-in-Dockerized-Application-83f5e66ee42a46c994d71571ae7ad44d . Thank you !!
@dishak
I have curated a plan of action for the issue. Can you please review it and let me know if I have understood the problem correctly and if any further improvements in the plan of action further. Once if the proposal is appealing I will go forward with the implementation. Here is the link of proposal : https://tan-shampoo-b5f.notion.site/Proposal-for-Using-pre-commit-in-Dockerized-Application-83f5e66ee42a46c994d71571ae7ad44d . Thank you !!
Proposal for Using
pre-commit
in Dockerized ApplicationProblem Description:
Implementing
pre-commit
hooks effectively within a Dockerized application to ensure consistent code quality and adherence to standards during development.
Plan 1: Integration of
pre-commit
Inside Docker ContainerProposed Plan of Action:
* Modify
docker-compose.yml
**:Update the
docker-compose.yml
file to includepre-commit
as part of the service definition for seamless integration.version: '3' services: my-service: build: . volumes: - .:/app # Mount current directory to /app inside the container command: pre-commit run --all-files
* Initialize Git Repository**:
Configure the
my-service
container to initialize a Git repository upon startup. Use appropriate bind mounts to persist Git data across container restarts.This approach automates the application of
pre-commit
hooks without manual intervention. Developers simply rundocker-compose up
to start the application withpre-commit
hooks applied.
My concerns with this approach:
command:
to pre-commit or create an additional container. The existing app
container could be configured to support pre-commit.git commit
) would need to be executed as docker command (ex. docker compose exec app git commit
).Plan 2: Leveraging Hot Reloading Tools and Running
pre-commit
LocallyProposed Plan of Action:
* Utilize Hot Reloading Tools**:
Implement hot reloading tools and bind mounts within the
app
service to detect code changes dynamically. This avoids the need to restart containers frequently during development.Example
docker-compose.yml
snippet:version: '3' services: my-service: build: . volumes: - .:/app # Mount current directory to /app inside the container command: uvicorn app:app --reload
*
pre-commit
will be executed locally**There are pros and cons which can be of course discussed but, please let me know if I have got hold of the problem and solution draft if is right approach to the problem.
My concerns with this approach:
app
container (which uses Django) already supports hot reloading.pipenv
.Thank you for writing up the proposal. Having additional methods documented helps me organize my own thinking. I think this issue, as it is currently written, makes too many assumptions and/or doesn't properly document the different contexts the app is used in.
Description
The docker environment is the primary supported development environment. It offers better parity with production and testing than a purely local pipenv and related services. However, the pre-commit hooks (as currently documented in
README.md
) act against the local repository, not the docker environment.Expectation
If pre-commit cannot be configured to work with the Docker environment, it should be dropped.
Regardless of wether pre-commit is dropped or its configuration is updated, the GitHub action should be updated to better minimize the delta between what is done in developer's environments and what is done via GitHub Actions.
Configurations
Dockerfile
docker-compose.yml
main/.pre-commit-config.yaml
.github/workflows/pre-commit.yml
Additional context