Skip to content

refactor: omit unnecessary reassignment#7

Merged
maoaixiao1314 merged 1 commit intohetu-project:mainfrom
weifangc:main
Dec 10, 2025
Merged

refactor: omit unnecessary reassignment#7
maoaixiao1314 merged 1 commit intohetu-project:mainfrom
weifangc:main

Conversation

@weifangc
Copy link
Contributor

@weifangc weifangc commented Nov 24, 2025

Description

The new version of Go has been optimized, and variables do not need to be reassigned.

For more info: https://tip.golang.org/wiki/LoopvarExperiment#does-this-mean-i-dont-have-to-write-x--x-in-my-loops-anymore


Closes #XXX

Summary by CodeRabbit

  • Tests
    • Simplified test loop variable handling across multiple test suites, removing redundant variable reassignments in test iterations. Changes applied to genesis, keeper, and client test files throughout the codebase.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: weifangc <weifangcheng@outlook.jp>
@coderabbitai
Copy link

coderabbitai bot commented Nov 24, 2025

Walkthrough

Remove the tc := tc loop variable rebinding pattern from 11 test files across multiple modules. This idiom was previously used to create local copies of loop variables inside range-based loops before closure capture in subtests, preventing closure capture issues in Go.

Changes

Cohort / File(s) Summary
Epochs
x/epochs/types/genesis_test.go
Removed loop variable shadowing in TestValidateGenesis
ERC20
x/erc20/keeper/msg_server_test.go, x/erc20/types/genesis_test.go
Removed loop variable rebinding in msg_server_test and genesis_test
EVM
x/evm/client/cli/utils_test.go, x/evm/keeper/msg_server_test.go, x/evm/types/genesis_test.go, x/evm/types/logs_test.go, x/evm/types/storage_test.go
Removed loop variable shadowing across multiple test files (5 instances in genesis_test and storage_test, 2 instances in logs_test)
Feemarket
x/feemarket/types/genesis_test.go
Removed loop variable rebinding in TestValidateGenesis
Inflation
x/inflation/keeper/msg_server_test.go, x/inflation/types/genesis_test.go
Removed loop variable shadowing in keeper and types test files
Vesting
x/vesting/keeper/integration_test.go
Removed loop variable shadowing in two for-range loops before It closures

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring extra attention:

  • x/evm/keeper/msg_server_test.go — Summary notes potential closure capture issue; verify that tc is correctly captured inside t.Run() subtests
  • x/vesting/keeper/integration_test.go — Summary explicitly flags risk of all tests using final loop value of tc instead of individual values; confirm test correctness post-removal
  • Go version compatibility — Verify whether this change assumes Go 1.22+ (which fixed the loop variable capture issue) or if it introduces subtle test flakiness
  • All remaining files — Cross-check that tests are not run in parallel mode where closure capture would be exposed

Poem

🐰 Loop shadows fade, like morning mist so clear,
Variable rebinds are captured without fear—
Each test now dances with the final tc's cheer,
Or freed by Go's new ways, let closures hold them dear!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'refactor: omit unnecessary reassignment' directly reflects the main change across all modified files: removing redundant tc := tc loop variable reassignments.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae8add9 and 08bdf60.

📒 Files selected for processing (12)
  • x/epochs/types/genesis_test.go (0 hunks)
  • x/erc20/keeper/msg_server_test.go (0 hunks)
  • x/erc20/types/genesis_test.go (0 hunks)
  • x/evm/client/cli/utils_test.go (0 hunks)
  • x/evm/keeper/msg_server_test.go (0 hunks)
  • x/evm/types/genesis_test.go (0 hunks)
  • x/evm/types/logs_test.go (0 hunks)
  • x/evm/types/storage_test.go (0 hunks)
  • x/feemarket/types/genesis_test.go (0 hunks)
  • x/inflation/keeper/msg_server_test.go (0 hunks)
  • x/inflation/types/genesis_test.go (0 hunks)
  • x/vesting/keeper/integration_test.go (0 hunks)
💤 Files with no reviewable changes (12)
  • x/erc20/keeper/msg_server_test.go
  • x/evm/types/genesis_test.go
  • x/feemarket/types/genesis_test.go
  • x/evm/client/cli/utils_test.go
  • x/inflation/types/genesis_test.go
  • x/vesting/keeper/integration_test.go
  • x/epochs/types/genesis_test.go
  • x/evm/types/storage_test.go
  • x/erc20/types/genesis_test.go
  • x/evm/types/logs_test.go
  • x/inflation/keeper/msg_server_test.go
  • x/evm/keeper/msg_server_test.go

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@weifangc
Copy link
Contributor Author

weifangc commented Dec 7, 2025

@maoaixiao1314 Hi, Could you please review this PR at your convenience? Thank you very much.

@maoaixiao1314 maoaixiao1314 merged commit c14217d into hetu-project:main Dec 10, 2025
4 checks passed
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