Skip to content

warning: unused variable #96786

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 1 commit into from
Closed

Conversation

chouetz
Copy link

@chouetz chouetz commented Sep 13, 2022

vi is unused in this code block according to my ide

vi is unused in this code block according to my ide
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@AlexWaygood
Copy link
Member

Thanks for your interest in improving CPython!

Unfortunately, cosmetic changes are usually not accepted for the CPython repo. We have a large number of open bug reports, and prefer to spend our time fixing those.

@chouetz
Copy link
Author

chouetz commented Sep 13, 2022

@AlexWaygood thanks for your comment, I wanted to do a separated pull-request from the modification I intend to do on bug gh-42811
Then I will do this cosmetic change embedded with this other PR

@AlexWaygood
Copy link
Member

@chouetz, we'd prefer it if you didn't do the cosmetic change at all, if that's okay :) including the change as part of a substantive PR will create a confusing diff and distract from the PR's main purpose

@chouetz
Copy link
Author

chouetz commented Sep 13, 2022

@AlexWaygood ok then we don't "fix" this kind of stuff, or only when noticed within a development that targets a precise block of code? Would it be ok if it's on a specific commit within a PR? Especially here because it's really trivial (I completely understand we don't want to bring confusion during a code review)
Sorry to ask such questions, just trying to do things right
Thanks in advance

@AlexWaygood
Copy link
Member

AlexWaygood commented Sep 13, 2022

Sorry to ask such questions, just trying to do things right

Of course, no worries!

Yeah, in general, we don't "fix" this kind of stuff:

  • Lots of the CPython codebase is really old and predates these kinds of style guides being written. If we encouraged PRs like this, there's unfortunately a risk that we'd get deluged with PRs "cleaning up" the CPython codebase, which isn't something we really want.
  • People have different opinions about what constitutes "good style", so encouraging PRs like this leads to annoying back-and-forths (E.g. I don't find the proposed change here significantly more readable, personally 🙂)
  • Cosmetic PRs lead to code churn and confusing git blame.

Every repo has their own policy on PRs like this; some are much more receptive than others; this is just the stance CPython has chosen to take here.

FYI, there's lots more information to new contributors that you might find useful in the devguide: https://devguide.python.org

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants