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

Move linting to pytest #5270

Closed
wants to merge 1 commit into from
Closed

Conversation

elazarg
Copy link
Contributor

@elazarg elazarg commented Jun 24, 2018

Use flake8's legacy api since there's no non-legacy one.

Another item for #1673 (only stdlibsamples left).

@gvanrossum
Copy link
Member

I'm not sure I like this. Maybe running flake8 should just be a separate step in .travis.yml? That's how it is done currently -- even though runtests.py supports it, .travis.yml excludes lint (and self-check) and runs flake8 separately. I want pytest to run tests not other stuff like linters and type-checkers. (FWIW it appears self-check is no long run by .travis.yml -- something has made python runtests.py package a no-op.)

@ilevkivskyi
Copy link
Member

Maybe running flake8 should just be a separate step in .travis.yml?

I agree with this. I would just remove flake8 from runtests.py

FWIW it appears self-check is no long run by .travis.yml

One can now run pytest -k mypy_package, I am not sure I like it, I would also prefer it to be a separate Travis (and Appveyor) task actually.

@elazarg
Copy link
Contributor Author

elazarg commented Jun 24, 2018

Sounds good to me. So this PR is not required at all - after stdlibsamples will be migrated, runtests.py and waiter.py can simple be removed. Is this correct?

@ilevkivskyi
Copy link
Member

after stdlibsamples will be migrated, runtests.py and waiter.py can simple be removed. Is this correct?

I think yes.

@elazarg
Copy link
Contributor Author

elazarg commented Jun 24, 2018

Closing then.

@elazarg elazarg closed this Jun 24, 2018
@gvanrossum
Copy link
Member

One can now run pytest -k mypy_package, I am not sure I like it, I would also prefer it to be a separate Travis (and Appveyor) task actually.

Yes, please.

@elazarg elazarg mentioned this pull request Jun 25, 2018
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.

None yet

3 participants