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

pylint fixes #3499

Closed
wants to merge 2 commits into from
Closed

pylint fixes #3499

wants to merge 2 commits into from

Conversation

marksmayo
Copy link

Fixes to a bunch of pylint warnings/fixes

Fixes to a bunch of pylint warnings/fixes
@marksmayo
Copy link
Author

Hmm, looks like a problem in the .tox folder, which I didn't modify...looking into it

Stupid typo fixed
@marksmayo
Copy link
Author

Duh, generated code from my code. Fixed typo.

@Zac-HD
Copy link
Member

Zac-HD commented Nov 2, 2022

I'm really sorry about this, because I can tell that this was a lot of work and I really appreciate that, but I don't think we can accept this PR 😥

Many of these changes are following rules which make sense in the abstract, but make the code harder to read (e.g. some of the else removals, removals of some parens), and others actually change the meaning of the code in ways that we don't want (e.g. changing x == [] to not x is a semantic change, and we often want the former). In many more cases, I just don't think it's worth the code churn and extra steps when searching through the git history.

If you're keen to contribute, I'd be delighted if you wanted to pick up an existing issue - #3478, #3131, or #3468 are good ones to start with, that don't require too much Hypothesis experience.

Again, apologies and thank you so much for the work you've clearly put in!

@Zac-HD Zac-HD closed this Nov 2, 2022
@marksmayo
Copy link
Author

:( :(

No that's ok, I totally get the reasoning. Might have a gander at the existing ones! :)

@Zac-HD
Copy link
Member

Zac-HD commented Nov 2, 2022

❤️

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