refactor: use the built-in max/min to simplify the code #1986
Conversation
Signed-off-by: carrychair <linghuchong404@gmail.com>
WalkthroughThe update simplifies the assignment of the Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
cmd/akash/cmd/testnetify/state.go(1 hunks)
| if vCount > uint32(len(sVals)) { | ||
| vCount = uint32(len(sVals)) | ||
| } | ||
| vCount := min(ga.app.StakingState.state.Params.MaxValidators, uint32(len(sVals))) |
There was a problem hiding this comment.
Undefined min function leads to a compile error
The new call to min(…) is not defined or imported anywhere in this file, resulting in an undefined identifier. Please add a helper for min(uint32, uint32) or revert to the manual comparison.
Proposed fixes:
- Define a helper at the top of the file:
// right after package testnetify
func min(a, b uint32) uint32 {
if a < b {
return a
}
return b
}- Or replace the single-line call with the original logic:
- vCount := min(ga.app.StakingState.state.Params.MaxValidators, uint32(len(sVals)))
+ vCount := ga.app.StakingState.state.Params.MaxValidators
+ if uint32(len(sVals)) < vCount {
+ vCount = uint32(len(sVals))
+ }🤖 Prompt for AI Agents
In cmd/akash/cmd/testnetify/state.go at line 999, the call to min(…) causes a
compile error because min is not defined or imported. Fix this by adding a
helper function min(a, b uint32) uint32 at the top of the file right after the
package declaration that returns the smaller of the two values, or alternatively
replace the min call with an inline conditional expression comparing the two
values manually.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Go version requirement in go.mod
echo "Checking Go version requirement in go.mod:"
cat go.mod | grep -E "^go [0-9]+\.[0-9]+"
# Check if there's any implementation of min function in the codebase
echo -e "\nChecking for custom min function implementations:"
rg "func min" -t go
# Check for any imports that might provide a min function
echo -e "\nChecking imports that might provide min function:"
cat cmd/akash/cmd/testnetify/state.go | grep -E "import|\"math\"" -A 30
Length of output: 1755
@carrychair You're absolutely right! The min function is a built-in feature available in Go 1.21 and later, and your project is using Go 1.23.5 as specified in your go.mod file.
The automated review flagged this without considering that you're using a recent version of Go that provides this functionality. Your PR title "refactor: use the built-in max/min to simplify the code" perfectly describes what you're doing here - leveraging the language's built-in features to make the code cleaner and more readable.
This is a good improvement to the codebase - using language features instead of repetitive conditional logic makes the code more maintainable.
🧬 Code Graph Analysis Results
Relevant snippets from cmd/akash/cmd/testnetify/util.go (lines 14-52)
func GetGovGenesisStateFromAppState(cdc codec.JSONCodec, appState map[string]json.RawMessage) *govtypes.GenesisState {
var genesisState govtypes.GenesisState
if appState[govtypes.ModuleName] != nil {
cdc.MustUnmarshalJSON(appState[govtypes.ModuleName], &genesisState)
}
return &genesisState
}
func GetIBCGenesisStateFromAppState(cdc codec.JSONCodec, appState map[string]json.RawMessage) *ibccoretypes.GenesisState {
var genesisState ibccoretypes.GenesisState
if appState[ibchost.ModuleName] != nil {
cdc.MustUnmarshalJSON(appState[ibchost.ModuleName], &genesisState)
}
return &genesisState
}
func GetSlashingGenesisStateFromAppState(cdc codec.JSONCodec, appState map[string]json.RawMessage) *slashingtypes.GenesisState {
var genesisState slashingtypes.GenesisState
if appState[slashingtypes.ModuleName] != nil {
cdc.MustUnmarshalJSON(appState[slashingtypes.ModuleName], &genesisState)
}
return &genesisState
}
func GetDistributionGenesisStateFromAppState(cdc codec.JSONCodec, appState map[string]json.RawMessage) *distributiontypes.GenesisState {
var genesisState distributiontypes.GenesisState
if appState[distributiontypes.ModuleName] != nil {
cdc.MustUnmarshalJSON(appState[distributiontypes.ModuleName], &genesisState)
}
return &genesisState
}Relevant snippet from x/market/alias.go (line 13)
ModuleName = types.ModuleNameRelevant snippet from cmd/akash/cmd/testnetify/config.go (lines 13-33)
AccAddress struct {
sdk.AccAddress
}
ValAddress struct {
sdk.ValAddress
}
ConsAddress struct {
sdk.ConsAddress
}
PubKey struct {
cryptotypes.PubKey
}Relevant snippet from testutil/ids.go (lines 25-29)
func AccAddress(t testing.TB) sdk.AccAddress {
t.Helper()
privKey := ed25519.GenPrivKey()
return sdk.AccAddress(privKey.PubKey().Address())
}Relevant snippet from testutil/sdk.go (lines 9-12)
func Coin(t testing.TB) sdk.Coin {
t.Helper()
return sdk.NewCoin("testcoin", sdk.NewInt(int64(RandRangeInt(1, 1000)))) // nolint: gosec
}Relevant snippet from testutil/base.go (lines 24-26)
func Name(_ testing.TB, prefix string) string {
return fmt.Sprintf("%s-%v", prefix, rand.Uint64())
}Relevant snippet from testutil/network/network.go (lines 134-153)
Validator struct {
AppConfig *srvconfig.Config
ClientCtx sdkclient.Context
Ctx *server.Context
Dir string
NodeID string
PubKey cryptotypes.PubKey
Moniker string
APIAddress string
RPCAddress string
P2PAddress string
Address sdk.AccAddress
ValAddress sdk.ValAddress
RPCClient tmclient.Client
tmNode *node.Node
api *api.Server
grpc *grpc.Server
grpcWeb *http.Server
}Relevant snippet from util/runner/runner.go (lines 7-13)
func Do(task Task) <-chan Result {
ch := make(chan Result, 1)
go func() {
ch <- task()
}()
return ch
}These snippets provide context on:
- How genesis states for various modules (gov, ibc, slashing, distribution) are unmarshaled from app state JSON.
- Aliases and wrapper structs for addresses and public keys used in the testnetify package.
- Utility functions for generating test addresses, coins, and names.
- The structure of a Validator in the test network setup.
- A utility function for running tasks asynchronously.
This context is relevant for understanding the new feature in state.go related to genesis state management, account and validator creation, and state packing/unpacking.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
|
Marked as stale; will be closed in five days. |
|
friendly ping~ |
|
Marked as stale; will be closed in five days. |
1 participant
Description
use the built-in max/min to simplify the code
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdSummary by CodeRabbit