Skip to content

Upgrading number of PyLint jobs from 1 to 10. #2401

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.

Sign up for GitHub

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

Jump to bottom
Closed
dhermes wants to merge 1 commit into from
Closed

Upgrading number of PyLint jobs from 1 to 10. #2401

dhermes wants to merge 1 commit into from

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Sep 23, 2016

Marked "don't merge" for now because I want to actually time the difference on the entire codebase. Working on it now.

@dhermes dhermes added testing hygiene do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Sep 23, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 23, 2016
@tseaver
Copy link
Contributor

tseaver commented Sep 23, 2016

LGTM

@dhermes
Copy link
Contributor Author

dhermes commented Sep 23, 2016

@tseaver May not be worth it.

At the current HEAD in master (or it was current until #2400 was merged)

$ git log -1 --pretty=%H
64353b5639a03ce9d2538eaf5fc112ea9019662e
$ rm -fr .tox/lint
$ time tox -e lint
...
real    3m17.283s
user    3m7.844s
sys     0m8.716s

at the HEAD in this PR

$ git log -1 --pretty=%H
dad810adf293f06dea50a459095a0e60924eb26f
$ rm -fr .tox/lint
$ tox -e lint --recreate --notest
$ time tox -e lint
...
real    3m26.578s
user    3m15.860s
sys     0m18.952s
$ time tox -e lint
...
real    3m21.792s
user    3m11.124s
sys     0m18.400s

So the extra jobs either

  • Add overhead without adding speed
  • Don't actually get set unless I do something else

@dhermes
Copy link
Contributor Author

dhermes commented Sep 23, 2016

pylint-dev/pylint#479

@tseaver
Copy link
Contributor

tseaver commented Sep 23, 2016

That PyCQA issue is supposed to be long-ago fixed.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 7, 2016

@tseaver I'm pretty sure we don't get the speed-up because of the changes introduced in #1974 (I was on paternity leave at the time so didn't fully understand the change). In particular, the fact that we run Pylint on each file separately.

@dhermes
Copy link
Contributor Author

dhermes commented Nov 10, 2016

Will revisit this some other time.

@dhermes dhermes closed this Nov 10, 2016
@dhermes dhermes deleted the update-pylint-config-round-3 branch November 10, 2016 23:59
parthea added a commit that referenced this pull request Nov 24, 2025
@renovate-bot @parthea
chore(deps): update all dependencies (#2401)
69b49f6 
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@tseaver tseaver

Labels

cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing. testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dhermes @tseaver @daspecster @googlebot