Skip to content
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

Add pre-commit to project #28

Merged
merged 3 commits into from
Mar 7, 2024
Merged

Add pre-commit to project #28

merged 3 commits into from
Mar 7, 2024

Conversation

brueggemann
Copy link
Contributor

I think we should add pre-commit to enforce black and probably other formatting/linting to pre-commit hooks.
After that some Makefile should be implemented to setup the dev environment and pre-commit install should be called there.

@brueggemann brueggemann marked this pull request as draft March 6, 2024 16:13
@brueggemann
Copy link
Contributor Author

@NotTheEvilOne do you know any best practices for a requirements.txt that is only needed in dev environments? Because this PR will add pre-commit (and its dependencies) to requirement.txt. I think this is not what we want

@brueggemann brueggemann self-assigned this Mar 6, 2024
@NotTheEvilOne
Copy link
Contributor

Yes, requirements.txt is for the "setuptools" based build process to define all runtime requirements. Based on PEP 621 there's no defined way to accomplish this requirement as part of a "development tool set for the project".

As "pre-commit" is only relevant for Git and this upstream effort I would suggest to check for the dependency in the pre-commit Git hook itself and add a sensible error message otherwise.

@brueggemann brueggemann force-pushed the prs/add-pre-commit-config branch 2 times, most recently from b46d412 to fa35892 Compare March 7, 2024 11:36
@brueggemann brueggemann marked this pull request as ready for review March 7, 2024 11:37
@brueggemann brueggemann force-pushed the prs/add-pre-commit-config branch 2 times, most recently from f5a6e3c to 34420c9 Compare March 7, 2024 11:43
Signed-off-by: Jan-Marten Brüggemann <[email protected]>
Signed-off-by: Jan-Marten Brüggemann <[email protected]>
Signed-off-by: Jan-Marten Brüggemann <[email protected]>
@brueggemann brueggemann merged commit 14541a6 into main Mar 7, 2024
2 checks passed
@brueggemann brueggemann deleted the prs/add-pre-commit-config branch March 7, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants