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

python: add flake8/pylint checking #816

Merged
merged 5 commits into from Dec 29, 2016

Conversation

Projects
None yet
4 participants
@trws
Copy link
Member

trws commented Sep 22, 2016

This is the start of the changes mentioned in #807 to have lints for the python bindings. I've been using flake8 as a start, because it's less freaked out by the generated modules. So far this reformats most of the bindings, but it's waiting for the merge of #799 rather than conflicting on code that's just going to be removed. As soon as everything passes, I'll add another commit in here to run flake8 or pylint with make check if it's available.

@garlick garlick added the review label Sep 22, 2016

@trws trws force-pushed the trws:syntax-check branch from 846fa2d to b839e2f Sep 22, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 22, 2016

Coverage Status

Coverage increased (+0.02%) to 75.296% when pulling b839e2f on trws:syntax-check into 914fb9f on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 22, 2016

Current coverage is 76.07% (diff: 100%)

Merging #816 into master will decrease coverage by <.01%

@@             master       #816   diff @@
==========================================
  Files           149        149          
  Lines         25951      25951          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          19743      19742     -1   
- Misses         6208       6209     +1   
  Partials          0          0          

Powered by Codecov. Last update b26fbff...8c32486

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Dec 22, 2016

This PR is getting a bit crusty - if it's on hold, how about closing and resubmitting when it is ready for another pass?

@trws trws force-pushed the trws:syntax-check branch 2 times, most recently from 30ef8bf to c3168d5 Dec 27, 2016

@trws

This comment has been minimized.

Copy link
Member Author

trws commented Dec 27, 2016

This should be complete now. The amount of tweaking and refactoring required to make pylint pass the code was more significant than I would have expected, but it's in now, and should remain enforced by make check henceforth. I selected pylint over flake8 because it's available by default on LC machines.

@trws trws force-pushed the trws:syntax-check branch from c3168d5 to a730dd0 Dec 27, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 28, 2016

Coverage Status

Coverage decreased (-0.01%) to 76.347% when pulling a730dd0 on trws:syntax-check into b26fbff on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Dec 28, 2016

Great!

One suggestion: break out the "enforce" part to its own commit. It gets lost in there with all the "refactor" changes.

Is it possible to install pylint in travis ?

@trws

This comment has been minimized.

Copy link
Member Author

trws commented Dec 28, 2016

@trws trws force-pushed the trws:syntax-check branch from a730dd0 to ea900ee Dec 29, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage decreased (-0.007%) to 76.354% when pulling ea900ee on trws:syntax-check into b26fbff on flux-framework:master.

@trws trws force-pushed the trws:syntax-check branch from ea900ee to 94caee6 Dec 29, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage increased (+0.04%) to 76.399% when pulling 94caee6 on trws:syntax-check into b26fbff on flux-framework:master.

@trws trws force-pushed the trws:syntax-check branch from 94caee6 to 0875e9b Dec 29, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage remained the same at 76.362% when pulling 0875e9b on trws:syntax-check into b26fbff on flux-framework:master.

trws added some commits Sep 22, 2016

python: refactor to pass pylint checks
All code in the flux module now passes pylint checks, though some are
locally disabled.
python: enforce that python module passes pylint
If "pylint" is in the path, make check will run it against the python
bindings.  Currently, it will cause a build failure on violation of any
python lint rules, including those that are convention or refactoring
messages, be wary.

@trws trws force-pushed the trws:syntax-check branch from 5234827 to 8c32486 Dec 29, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage increased (+0.03%) to 76.388% when pulling 5234827 on trws:syntax-check into b26fbff on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage decreased (-0.004%) to 76.358% when pulling 8c32486 on trws:syntax-check into b26fbff on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Dec 29, 2016

OK this looks good. Thanks @trws!

@garlick garlick merged commit dbd8883 into flux-framework:master Dec 29, 2016

4 checks passed

codecov/patch Coverage not affected when comparing b26fbff...8c32486
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +23.92% compared to b26fbff
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.004%) to 76.358%
Details

@garlick garlick removed the review label Dec 29, 2016

@grondo grondo referenced this pull request Mar 28, 2017

Closed

0.7.0 Release Notes #1019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.