Skip to content

Utilize pre-commit to help devs follow style guidelines #1495

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Aug 28, 2021
Merged

Utilize pre-commit to help devs follow style guidelines #1495

merged 5 commits into from
Aug 28, 2021

Conversation

tim-schilling
Copy link
Member

Closes #1485

@matthiask one thing that requires a close look is .pre-commit-config.yaml. Let me know if I should remove any that you think are overly strict. I picked from https://pre-commit.com/hooks.html and matched what we had. I did introduce doc8 since it seems like a positive change.

@codecov
Copy link

codecov bot commented Aug 24, 2021

Codecov Report

Merging #1495 (30e5149) into main (3ed59cb) will increase coverage by 0.02%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1495      +/-   ##
==========================================
+ Coverage   86.58%   86.61%   +0.02%     
==========================================
  Files          35       35              
  Lines        1864     1868       +4     
  Branches      262      262              
==========================================
+ Hits         1614     1618       +4     
  Misses        178      178              
  Partials       72       72              
Impacted Files Coverage Δ
debug_toolbar/panels/cache.py 82.26% <0.00%> (ø)
debug_toolbar/management/commands/debugsqlshell.py 100.00% <100.00%> (ø)
debug_toolbar/panels/history/__init__.py 100.00% <100.00%> (ø)
debug_toolbar/panels/sql/__init__.py 100.00% <100.00%> (ø)
debug_toolbar/panels/templates/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ed59cb...30e5149. Read the comment docs.

Copy link
Member

@matthiask matthiask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, especially the strictness of it :)

Regarding F401: You could add e.g. __all__ = ["SQLPanel"] to avoid the need to silence this particular error, but it's definitely fine as it is.

@tim-schilling
Copy link
Member Author

I'm glad I forced the specific flake8 warnings, TIL. I'll make that change and merge it in.

@tim-schilling tim-schilling merged commit 6d3eb44 into django-commons:main Aug 28, 2021
@tim-schilling tim-schilling deleted the pre-commit branch August 28, 2021 15:30
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup pre-commit for repo
2 participants