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.

Sign up for GitHub

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

Jump to bottom

cmd/gofmt: inconsistent variable names in rewrite.go #57567

Closed
rillig opened this issue Jan 4, 2023 · 2 comments
Closed

cmd/gofmt: inconsistent variable names in rewrite.go #57567

rillig opened this issue Jan 4, 2023 · 2 comments
Labels
FrozenDueToAge help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@rillig
Copy link
Contributor

rillig commented Jan 4, 2023

What did you do?

Read through cmd/gofmt/rewrite.go in order to find out how to deal with comments when instrumenting Go code.

What did you expect to see?

The Values/types for special cases use a consistent naming scheme.

What did you see instead?

  • Some of the variable names include Ptr, others omit it. Either all or none should include the word Ptr.
  • positionType should rather be posType, as it is about token.Pos, not token.Position.
@heschi heschi added help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jan 4, 2023
@heschi
Copy link
Contributor

heschi commented Jan 4, 2023

Do you intend to fix this? If so, cleanup CLs don't generally need an issue associated. If not, I'm not sure this is worth tracking as an issue.

cc @griesemer

@heschi heschi added this to the Backlog milestone Jan 4, 2023
@griesemer
Copy link
Contributor

griesemer commented Jan 4, 2023 •
edited

There is no issue here. These naming choices have been made many years ago, and minor inconsistencies in naming is no reason to make changes in this code.

@golang golang locked and limited conversation to collaborators Jan 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@rillig @griesemer @gopherbot @heschi