Skip to content

Conversation

@Kami
Copy link
Member

@Kami Kami commented Feb 12, 2017

I noticed it was broken when I was upgrading pylint as part of #3189.

We passed invalid (in-existent) path to pylint but older versions simply silently ignored that.

For example, it got passed in contrib/runners/action_chain_runner/contrib/runners/action_chain_runner/ instead of contrib/runners/action_chain_runner/ so files in those directories were not actually pylinted.

Kami added 2 commits February 12, 2017 17:42
because pylint simply silently ignores invalid / inexistent paths.
@Kami
Copy link
Member Author

Kami commented Feb 12, 2017

/cc @bigmstone

Kami added 2 commits February 12, 2017 18:02
Pylint either requires path to a package or a module, passing a path to
a directory which is not a package doesn't work and such directory is
simply skipped so we need to pass paths to actual Python files /
modules.
@codecov-io
Copy link

codecov-io commented Feb 12, 2017

Codecov Report

Merging #3215 into master will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3215      +/-   ##
==========================================
+ Coverage   77.64%   77.71%   +0.07%     
==========================================
  Files         433      433              
  Lines       22414    22414              
==========================================
+ Hits        17402    17418      +16     
+ Misses       5012     4996      -16
Impacted Files Coverage Δ
st2common/st2common/models/db/runner.py 100% <ø> (ø)
st2reactor/st2reactor/garbage_collector/config.py 0% <ø> (ø)
st2common/st2common/models/db/rule.py 100% <ø> (ø)
st2common/st2common/models/db/rule_enforcement.py 76.67% <ø> (ø)
st2common/st2common/log.py 86.67% <ø> (ø)
st2common/st2common/content/bootstrap.py 0% <ø> (ø)
st2api/st2api/controllers/resource.py 87.19% <ø> (+0.41%)
st2common/st2common/models/db/sensor.py 100% <ø> (ø)
st2common/st2common/models/db/marker.py 100% <ø> (ø)
st2common/st2common/models/db/executionstate.py 100% <ø> (ø)
... and 23 more

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 74d3dbd...bb98bed. Read the comment docs.

@Kami
Copy link
Member Author

Kami commented Feb 12, 2017

Discovered another issue - f1cf556.

Copy link
Contributor

@lakshmi-kannan lakshmi-kannan left a comment

Choose a reason for hiding this comment

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

Good catch +1

@Kami
Copy link
Member Author

Kami commented Feb 14, 2017

Re line break change - it was needed because of new version of flake8 which improves detection of "2 line breaks after a class and function definition" rule.

This rule was there from the beginning, but it didn't correctly detect all the places and new version of flake8 fixes that so we need to update our code.

@Kami
Copy link
Member Author

Kami commented Feb 15, 2017

Since we will be merging #3219 into master, I will go ahead and merge this one as well.

This one doesn't affect st2 code, just the build and improves lint detection.

@Kami
Copy link
Member Author

Kami commented Feb 15, 2017

e2e test failures are related to external service being offline.

StackStorm/st2cd#243 should hopefully improve the situation.

@Kami Kami merged commit ad69cc8 into master Feb 15, 2017
@Kami Kami deleted the fix_runners_pylint branch February 15, 2017 13:37
@Kami Kami mentioned this pull request Feb 15, 2017
1 task
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.

4 participants