Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: remove unnecessary redeclarations in for loops#18440

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
aslilac wants to merge3 commits intomain
base:main
Choose a base branch
Loading
fromlilac/go-122-range-fix

Conversation

aslilac
Copy link
Member

disclaimer: I asked claude 4 to do this little bit of cleanup.

I noticed we still have a bunch of these, but these kinds of re-declarations are no longer necessary now that we live in a post go 1.22 world. not a huge change, but didn't take claude very long. :p

Copy link
Member

@johnstcnjohnstcn left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I checked and as far as I can tell this should all be safe. If we wanted to be super-safe, we'd just do the ones in*_test.go and leave the rest till after code freeze.
There are still a few straggling comments left that can also probably be removed.

@aslilacaslilacforce-pushed thelilac/go-122-range-fix branch from40fd4ac tobf077d3CompareJune 18, 2025 22:05
@aslilac
Copy link
MemberAuthor

sorry about that, didn't expect anyone to look so quickly. just cleaned the diff up a bunch.

@aslilacaslilac changed the titlechore: remove unnecessary redelarations in for loopschore: remove unnecessary redeclarations in for loopsJun 18, 2025
johnstcn
johnstcn previously approved these changesJun 19, 2025
Copy link
Member

@johnstcnjohnstcn left a comment•
edited
Loading

Choose a reason for hiding this comment

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

All of these loopvar instances look safe to remove.

EDIT: for an abundance of safety, I'd prefer if weonly did this for*_test.go and left the rest alone.

dannykopping reacted with thumbs up emoji
Comment on lines -411 to -413
// When we assign the non-pointer to a
// variable it loses the reference.
replica := replica
Copy link
Member

Choose a reason for hiding this comment

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

This is the only instance I'm slightly wary of, but the new behaviour seems to be as intended here. It was originally added in#4596 to avoid a test race.

@johnstcnjohnstcn dismissed theirstale reviewJune 19, 2025 08:06

*_test.go please

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@johnstcnjohnstcnjohnstcn left review comments

@spikecurtisspikecurtisAwaiting requested review from spikecurtisspikecurtis is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@aslilacaslilac

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@aslilac@johnstcn
[8]ページ先頭
©2009-2025 Movatter.jp