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

Pass tox posargs to pylint #15226

Merged
merged 1 commit into from
Jul 2, 2018
Merged

Pass tox posargs to pylint #15226

merged 1 commit into from
Jul 2, 2018

Conversation

scop
Copy link
Member

@scop scop commented Jun 30, 2018

Description:

Being able to pass arguments to pylint is useful, for example --jobs=N, -f parseable etc.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

@homeassistant homeassistant added cla-signed small-pr PRs with less than 30 lines. labels Jun 30, 2018
@ghost ghost added the in progress label Jun 30, 2018
@scop
Copy link
Member Author

scop commented Jun 30, 2018

I also experimented with setting the number of jobs to various values locally and in Travis.

In Travis, it seems setting it to anything but 1 just slows things down.

Locally for me (dual core hyperthreading laptop --> 4 cpus) good values include 2 or 3 jobs. 4 (i.e. the same as 0) makes the system too unresponsive for other work, if not run with nice, and with nice, gives pretty much same results as 3 without it, with similar effect on responsiveness.

So, worth considering in addition to this change would be to add jobs=0 or jobs=2 to pylintrc, and forcing Travis to serial linting with -- --jobs=1: 0 would bring largish speedups at cost of making system unresponsive, 2 would be a rather conservative default that would give ~2x speedup on most systems, while possibly hurting lower powered ones to some extent. In any case, the default can be overridden using --jobs=N on the command line. Thoughts, should something like this be additionally done? Personally somewhat torn over this, perhaps somewhat leaning towards not trying anything past this commit but leaving it to local invocations, e.g. nice tox -e pylint -- --jobs=0.

@andrey-git
Copy link
Contributor

Thanks!
Lets merge this as-is meanwhile.

@andrey-git andrey-git merged commit 4d93a9f into home-assistant:dev Jul 2, 2018
@ghost ghost removed the in progress label Jul 2, 2018
@scop scop deleted the pylint-posargs branch July 2, 2018 14:35
awarecan pushed a commit to awarecan/home-assistant that referenced this pull request Jul 16, 2018
@balloob balloob mentioned this pull request Jul 20, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants