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

Use pre-commit #347

Closed
wants to merge 2 commits into from
Closed

Conversation

kurtmckee
Copy link
Contributor

git pre-commit hooks can catch linting issues on developer PCs before they're ever committed to the repo. pre-commit is a widely-used tool that allows projects to specify all of the linters they want to run before each commit.

This PR introduces a pre-commit config that will enforce standard text linting; black, isort, and editorconfig linting; and idiomatic Python 3.6+ syntax.

  • Developers with the pre-commit tool installed can simply run pre-commit install in the dotbot repo, and from that point forward git will run the tool prior to each commit so the files are linted and auto-fixed.
  • The dotbot repo can be registered with the pre-commit.ci service so that it auto-checks -- and, by default, autofixes! -- incoming PRs. It will also open PRs to keep the pre-commit hook versions updated on a regular basis. I STRONGLY recommend doing this!

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2023

Codecov Report

Patch coverage: 80.55% and project coverage change: +0.01% 🎉

Comparison is base (840cd16) 89.13% compared to head (5246ca2) 89.14%.
Report is 4 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
+ Coverage   89.13%   89.14%   +0.01%     
==========================================
  Files          33       33              
  Lines        1620     1622       +2     
  Branches      306      307       +1     
==========================================
+ Hits         1444     1446       +2     
  Misses        124      124              
  Partials       52       52              
Files Changed Coverage Δ
dotbot/cli.py 67.36% <0.00%> (ø)
dotbot/util/string.py 25.00% <0.00%> (ø)
dotbot/plugins/link.py 86.95% <50.00%> (ø)
dotbot/messenger/messenger.py 65.11% <100.00%> (ø)
dotbot/plugins/clean.py 91.30% <100.00%> (ø)
dotbot/plugins/create.py 75.60% <100.00%> (ø)
dotbot/plugins/shell.py 94.82% <100.00%> (ø)
dotbot/util/common.py 100.00% <100.00%> (ø)
dotbot/util/singleton.py 100.00% <100.00%> (ø)
tests/conftest.py 92.35% <100.00%> (+0.09%) ⬆️
... and 4 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kurtmckee
Copy link
Contributor Author

If unfamiliar with pre-commit, you can do the following to test it out:

  • Checkout this branch
  • Install pre-commit (for example, in an activated virtual environment using pip install pre-commit or using pipx -- which I strongly recommend -- using pipx install pre-commit)
  • Enable pre-commit in the dotbot repo (run pre-commit install)
  • Modify two files by rearranging their imports.
  • Stage one of the two files (but not both) for a commit.
  • Try to commit the file.

What you'll see is that pre-commit runs isort against the one staged file (but not both!), isort lints and fixes the file's imports, and the commit fails. Notably, pre-commit stashes changes temporarily so only what you've staged gets checked.

For example, this is the output I see locally when I rearrange the imports in cli.py and another file, but only stage cli.py in a commit:

[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to C:\Users\Kurt\.cache\pre-commit\patch1694373671-15164.
check yaml...........................................(no files to check)Skipped
fix end of files.........................................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
Enforce Python 3.6 idioms................................................Passed
black....................................................................Passed
isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing C:\Users\Kurt\Documents\dev\dotbot\dotbot\cli.py

Check .editorconfig rules................................................Passed
[INFO] Restored changes from C:\Users\Kurt\.cache\pre-commit\patch1694373671-15164.

I have to stage isort's fixes to my commit before I can successfully commit.

@kurtmckee
Copy link
Contributor Author

Hi Anish! I hope your year is off to a good start!

I'm going to close this PR to reduce cognitive load.

Thanks for your work on dotbot!

@kurtmckee kurtmckee closed this Mar 5, 2024
@kurtmckee kurtmckee deleted the use-pre-commit branch March 5, 2024 22:53
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