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

Apply refurb suggestion #939

Merged
merged 3 commits into from
Dec 5, 2022
Merged

Apply refurb suggestion #939

merged 3 commits into from
Dec 5, 2022

Conversation

DimitriPapadopoulos
Copy link
Contributor

  • [FURB102]: Replace x.endswith(y) or x.endswith(z) with x.endswith((y, z))

[FURB102]: Replace `x.endswith(y) or x.endswith(z)` with `x.endswith((y, z))`
@di
Copy link
Sponsor Member

di commented Nov 30, 2022

What's the reasoning behind this change?

@DimitriPapadopoulos
Copy link
Contributor Author

This commit applies a refurb suggestion - refurb is a linter. Not all suggestions from linters make sense, I just felt this one was making sense. This commit does not fix any bug.

Copy link
Contributor

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

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

I'm definitely a fan of idiomatic Python, and refurb seems well-intentioned on that front. I'm not sure it's worth adding another linter, but I don't mind merging this one-off. Thanks!

@bhrutledge bhrutledge enabled auto-merge (squash) December 5, 2022 01:48
@bhrutledge bhrutledge merged commit f9461a4 into pypa:main Dec 5, 2022
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Dec 5, 2022

I am not certain refurb is mature enough to be added as a CI linter. I feel some of the suggestions result in less readable code, especially for corner cases.

But then so I had initially the same feeling with black, and now while I am not necessarily a fan of black, I find code formatted by black more readable and I like not having to think about formatting.

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