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

Skip unchanged files which is checked #83

Merged
merged 5 commits into from
Jan 15, 2019
Merged

Skip unchanged files which is checked #83

merged 5 commits into from
Jan 15, 2019

Conversation

yanqd0
Copy link
Contributor

@yanqd0 yanqd0 commented Jan 5, 2019

The pylint is too slow, especially when the project is large.

pytest-flakes can skip unchanged checked files by pytest cache.
This implementation is learned from that.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 98.381% when pulling 6054701 on yanqd0:patch into ea48694 on carsongee:master.

@coveralls
Copy link

coveralls commented Jan 5, 2019

Coverage Status

Coverage increased (+0.5%) to 98.901% when pulling e48f4d9 on yanqd0:patch into 875d155 on carsongee:master.

Copy link
Owner

@carsongee carsongee left a comment

Choose a reason for hiding this comment

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

This is really exciting, thanks so much for the contribution and great idea!

So due to earlier performance speedups, we're actually running all the tests in one shot on the session collection finish: https://github.com/carsongee/pytest-pylint/pull/83/files#diff-b8aeb412a793e07c521aa3f51c31e245R207 from the list added here: https://github.com/carsongee/pytest-pylint/pull/83/files#diff-b8aeb412a793e07c521aa3f51c31e245R187 (or a couple lines down if there are ignores). So in this PR as it stands, pylint is still running

I'm thinking everything here is still all setup correct, I think the cleanest way to stop the test from actually running is to add a property to PyLintItem, something like should_run that we populate in __init__, and then rework the section (https://github.com/carsongee/pytest-pylint/pull/83/files#diff-b8aeb412a793e07c521aa3f51c31e245R187), to be something closer to:

item = PyLintItem(path, parent)
if item.should_run:
    session.pylint_files.add(rel_path)
return item

so that we keep all the nice class encapsulation, but don't run the linter.

@yanqd0
Copy link
Contributor Author

yanqd0 commented Jan 11, 2019

I agree. It is my mistake to ignore the real linting. Thanks for your guide.

The patch is rebased and your solution is implemented.

@carsongee
Copy link
Owner

Woot woot, thanks!

@carsongee carsongee merged commit a319ad4 into carsongee:master Jan 15, 2019
@yanqd0 yanqd0 deleted the patch branch January 15, 2019 10:52
@carsongee
Copy link
Owner

This is now released, thanks again!

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.

3 participants