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

GitHub Action to run pre-commit on all pull requests #311

Closed
wants to merge 4 commits into from

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Feb 25, 2022

This ensures that if a contributor fails to install pre-commit locally, CI will still highlight any issues in their submissions.

@akaihola akaihola added the CI label Feb 25, 2022
@akaihola
Copy link
Owner

This makes most sense if we move all checks from the workflow to pre-commit, and just run pre-commit from the workflow, right?

I'm a bit torn on this, since I also like GitHub Actions showing each different check as a separate item in the UI. Is there any way to make that still happen when doing the checks within pre-commit?

@cclauss
Copy link
Contributor Author

cclauss commented Feb 25, 2022

You understand well -- I too am torn about how to implement this but I tend to put the fastest checks in pre-commit and the slower checks in a separate GitHub Action.

https://pre-commit.ci is supercool because it allows for automatic corrections to be committed back to the PR but that is a whole other CI tool to consider.

@akaihola
Copy link
Owner

pre-commit documentation describes the pre-commit run option [hook-id]:

Options:

  • [hook-id]: specify a single hook-id to run only that hook.

Some example useful invocations:

  • [...]
  • pre-commit run flake8: run the flake8 hook against all staged files.

Using that option, we could still run each check as their own step in the GitHub Workflow, while in developers' environments git-commit would run them all.

That would of course require remembering to add new checks also to the workflow after adding them to the pre-commit configuration.

This ensures that if a contributor fails to install pre-commit, CI will highlight any issues in their submissions.
@cclauss
Copy link
Contributor Author

cclauss commented Feb 25, 2022

I find the pre-commit output to be quite readable and have never had any contributor complaints about not understanding that output.

@akaihola akaihola added this to the 1.5.0 milestone Mar 4, 2022
@akaihola
Copy link
Owner

akaihola commented Mar 20, 2022

For the 1.5.0 version, we'll keep running checks directly from GitHub workflows. Let's consider pre-commit in 1.6.0. Thanks again @cclauss!

@akaihola akaihola modified the milestones: 1.5.0, 1.6.0 Mar 20, 2022
@akaihola
Copy link
Owner

I've been experimenting with some GitHub actions which annotate the Files changed view of the pull request with messages from linters and checkers.

To combine that with a good pre-commit setup for local use, it seems to me that at least some configuration needs to be duplicated, which is a bit of a bummer.

@cclauss cclauss closed this Aug 28, 2022
@cclauss cclauss deleted the patch-2 branch August 28, 2022 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

2 participants