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

Add pylint in the continuous integration and pre-commit #7

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

Pierre-Sassoulas
Copy link
Member

Follow-up to #6

@Pierre-Sassoulas Pierre-Sassoulas added the enhancement New feature or request label Oct 14, 2023
@Pierre-Sassoulas Pierre-Sassoulas marked this pull request as draft October 14, 2023 12:50
@codecov
Copy link

codecov bot commented Oct 14, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (1c2613b) 86.96% compared to head (2fbc3db) 86.96%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master       #7   +/-   ##
=======================================
  Coverage   86.96%   86.96%           
=======================================
  Files          33       33           
  Lines         399      399           
  Branches       81       81           
=======================================
  Hits          347      347           
  Misses         48       48           
  Partials        4        4           
Files Coverage Δ
tests/base_tester.py 88.70% <100.00%> (ø)

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

@Pierre-Sassoulas Pierre-Sassoulas force-pushed the pre-commit-pylint branch 2 times, most recently from b6913ec to d1634fd Compare October 14, 2023 18:02
@Pierre-Sassoulas Pierre-Sassoulas marked this pull request as ready for review October 14, 2023 18:18
pyproject.toml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@stdedos stdedos added the CI label Oct 15, 2023
@stdedos stdedos added this to the Release 1.1.x milestone Oct 15, 2023
@stdedos
Copy link
Collaborator

stdedos commented Oct 15, 2023

idk why codecov is crying for, ignore it

Copy link
Collaborator

@stdedos stdedos left a comment

Choose a reason for hiding this comment

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

You have, at least, missed good-names from the old config.

Some of those names ofc, are coming from the pylint (<3?) limitation that two-lettered variables are not acceptable (I don't remember the issue number).

Let me contemplate on the configuration? Otherwise this is solid work!

@Pierre-Sassoulas
Copy link
Member Author

For good names everything is either the pylint default or something not required now that pylint does not raise for <3 letters variables anymore

-good-names=ex,Run,_,i,j,k, # Defaults
-           rc,             # Return variable of `subprocess.xxx` methods
-           df,             # Panda's DataFrame variable
-           cd,             # Method/Context function that does `cd`. `cwd` is not much better

Copy link
Collaborator

@stdedos stdedos left a comment

Choose a reason for hiding this comment

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

Latest changes bring over some of the things I like. I think this is done for now, yes.

@Pierre-Sassoulas Pierre-Sassoulas merged commit a444a61 into master Oct 17, 2023
43 checks passed
@Pierre-Sassoulas Pierre-Sassoulas deleted the pre-commit-pylint branch October 17, 2023 19:47
@Pierre-Sassoulas
Copy link
Member Author

Great !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants