Skip to content

Move pylint to pre-commit -> faster feedback for devs #4242

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

Merged
merged 2 commits into from
Nov 25, 2020

Conversation

MarcoGorelli
Copy link
Contributor

Currently, if a dev opens a PR, it can take half an hour for them to see the CI process fail because of some "unused import" error from pylint.

IMO pylint could be moved over to pre-commit, as it can be run in its own isolated environment - this way, devs can get faster feedback if they violate pylint rules. Even if they don't use pre-commit locally, they'll get feedback within 1-2 minutes from the CI job.

@@ -34,8 +34,7 @@ disable=all
# Enable the message, report, category or checker with the given id(s). You can
# either give multiple identifier separated by comma (,) or put this option
# multiple time. See also the "--disable" option for examples.
enable=import-error,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if pylint runs in an isolated virtualenv, then it won't be able to import third-party modules. However, invalid imports would already raise an error during the pytest jobs, so this check is redundant and can be removed

@codecov
Copy link

codecov bot commented Nov 22, 2020

Codecov Report

Merging #4242 (0b05244) into master (a6b13e7) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4242      +/-   ##
==========================================
+ Coverage   88.11%   88.19%   +0.08%     
==========================================
  Files          87       89       +2     
  Lines       14243    14358     +115     
==========================================
+ Hits        12550    12663     +113     
- Misses       1693     1695       +2     
Impacted Files Coverage Δ
pymc3/backends/base.py 86.92% <100.00%> (ø)
pymc3/distributions/multivariate.py 81.12% <0.00%> (-0.05%) ⬇️
pymc3/distributions/distribution.py 94.42% <0.00%> (-0.05%) ⬇️
pymc3/__init__.py 100.00% <0.00%> (ø)
pymc3/util.py 94.38% <0.00%> (ø)
pymc3/model.py 89.36% <0.00%> (+0.19%) ⬆️

@twiecki twiecki merged commit 2723b6c into pymc-devs:master Nov 25, 2020
@MarcoGorelli MarcoGorelli deleted the move-pylint-to-pre-commit branch November 25, 2020 19:00
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