Skip to content

Demo to see how pre-commit and reviewdog respond to flake8 error #12

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

Closed
wants to merge 7 commits into from

Conversation

matthewfeickert
Copy link
Owner

What do pre-commit.ci and reviewdog do when a flake8 error is introduced?

* Update pre-commit hooks:
   - https://github.com/pre-commit/pre-commit-hooks: v4.0.1 → v4.2.0
   - https://github.com/pycqa/flake8: v3.9.2 → v4.0.1

This is being applied manually and not waiting for pre-commit.ci to
apply it as pre-commit.ci hasn't been turned on yet since these
hooks were introduced in PR 21583.
Also remove comments in lib/matplotlib/backend_tools.py per request
of maintainer in PR review.
Add a git blame ignore-revs file to automatically ignore commits listed
in it from the blame view on GitHub. Include in it style changes added by
pre-commit hooks which touch multiple files.

To ignore the commits in the .git-blame-ignore-revs when running `git blame`
locally run: `git blame --ignore-revs-file .git-blame-ignore-revs`.

c.f.:
* https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt
* https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view
Add instructions to the devs on how to manually run pre-commit hooks, both for
all the hooks, and for individual hook ids. Example:
`pre-commit run flake8 --all-files`

Also use the 'python -m pip' pattern to be consistent with the rest of the docs.
@matthewfeickert
Copy link
Owner Author

matthewfeickert commented Apr 19, 2022

So to summarize:

Action pre-commit.ci reviewdog
Red X in check summary Yes Yes
Comment on failure No (autofix disabled in config) No (makes annotation with no additional alert to PR author)
Number of clicks to see failure and line number 1 1
Number of clicks to see failure in PR diff N/A 2
Able to correct errors on request Yes No
Automatically keep pre-commit hooks updated (on schedule set in config) Yes No
Requires maintaining a CI workflow No Yes

There is of course the option of implementing both, but if pre-commit.ci is turned on then unless there is significant benefit gained from reviewdog's annotations it seems like adding reviewdog is just adding maintenance costs to the dev team.

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.

1 participant