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 verification to CI #1072

Closed

Conversation

maichmueller
Copy link
Contributor

Builds on #1071.

Adds a CI step that verifies all pre-commit checks pass, otherwise fails the build step and exits. Not built on #1071 yet to keep diff small and the changes easily in sight. Will need #1071 merged into it to become operational.

Currently set as a condition for the build job to run, but can be set as a side job.

Merging this PR will mean that the entire codebase needs to apply the checks and formatting mandated by the pre-commit setup and as such requires one behemoth PR to apply these changes, otherwise all CI steps will constantly fail.

I do not recommend merging this until all open PRs in consideration for the project have been merged or closed so that the entire repo can see all of the pre-commit hooks applied to it.

@lanctot
Copy link
Collaborator

lanctot commented Oct 23, 2023

Hi @maichmueller, I think this is great, and in another world where I had enough time to properly look into it and understand how it all works, I would fully embrace the move. I realize it would probably save us time long term but I don't even have the time to spend on the initial cost of learning how it all works.

I also think this is a bit heavy for contributors and would rather keep doing what we have been and keep it easy to contribute.

@lanctot lanctot closed this Oct 23, 2023
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.

2 participants