Skip to content

Run flake8 via pre-commit #80

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

Closed
wants to merge 5 commits into from

Conversation

peternewman
Copy link
Contributor

@peternewman peternewman commented Jan 17, 2021

This is currently deliberately running without #77 in place to test and show the failing output.

@peternewman peternewman mentioned this pull request Jan 17, 2021
@pradyunsg
Copy link
Member

I don't think we need the annotation, or that we need to run pre-commit twice. Just once is enough.

@peternewman
Copy link
Contributor Author

I don't think we need the annotation,

Okay, I can remove it if you want, I think it's quite a nice feature personally and I've been trying to put it into any actions I use to make things more user-friendly for people.

or that we need to run pre-commit twice. Just once is enough.

Running flake8 as Python 2 and Python 3 will both produce different results. Given you're still targeting both, I think if it's only run once it will potentially miss some issues.

@pradyunsg
Copy link
Member

Given you're still targeting both, I think if it's only run once it will potentially miss some issues.

We've documented that we're dropping support for Python 2 this month in pip 21.0, and I'd rather we don't create busy work to do next week. :)

@peternewman
Copy link
Contributor Author

We've documented that we're dropping support for Python 2 this month in pip 21.0, and I'd rather we don't create busy work to do next week. :)

Ah sorry, I'd not seen that. I've now removed that bit from the config.

Base automatically changed from master to main March 3, 2021 18:17
@pradyunsg
Copy link
Member

Closing since this is significantly out of date and the whole automation has been rewritten in #85.

I still feel that there is very little value to linting the code in this repository via CI -- and it is basically useless for the generated scripts. It's likely fine to not do this, and any energy we put into updating this is probably better spent removing lines from https://github.com/pypa/pip/blob/master/.pre-commit-config.yaml#L23 (one by one in separate PRs, so that it's easier to review those PRs).

Thanks for working on this though @peternewman! Much appreciated! ^>^

@pradyunsg pradyunsg closed this Mar 12, 2021
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.

2 participants