-
Notifications
You must be signed in to change notification settings - Fork 24
Networks or ids #607
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
base: main
Are you sure you want to change the base?
Networks or ids #607
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update blockchain network selection and display logic in the CLI. The type for specifying blockchains is generalized, network selection prompts are simplified to only allow supported networks, and the display of network names now includes chain IDs and tier icons when available. Error handling and conversion logic are also improved. Changes
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
codegenerator/cli/src/cli_args/interactive_init/evm_prompts.rs (1)
9-9
: Clean up commented import.The commented-out
UniqueValueValidator
import should be removed if it's no longer needed.- // validation::UniqueValueValidator,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
codegenerator/cli/src/cli_args/clap_definitions.rs
(2 hunks)codegenerator/cli/src/cli_args/interactive_init/evm_prompts.rs
(3 hunks)codegenerator/cli/src/config_parsing/chain_helpers.rs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
🔇 Additional comments (6)
codegenerator/cli/src/cli_args/clap_definitions.rs (3)
171-171
: Import cleanup looks good.The removal of
NetworkWithExplorer
import aligns with the type change in theExplorerImportArgs
struct.
238-238
: Excellent improvement in flexibility.Changing from
Option<NetworkWithExplorer>
toOption<NetworkOrChainId>
allows users to specify networks either by name or chain ID, which is much more user-friendly.
253-282
: Well-designed enum with proper trait implementations.The
NetworkOrChainId
enum and its implementations are well-structured:
From<NetworkOrChainId> for u64
handles both variants correctlyFromStr
implementation properly tries network name first, then falls back to chain ID parsing- Error handling in
FromStr
is appropriate with contextual error messagescodegenerator/cli/src/config_parsing/chain_helpers.rs (1)
612-626
: Enhanced display format improves user experience.The updated
get_pretty_name
method provides much more context by including:
- Network name
- Chain ID in parentheses
- Tier icon for HypersyncNetworks (when available)
The fallback to just network name and chain ID for non-HypersyncNetworks is handled correctly. This will make network identification much clearer for users.
codegenerator/cli/src/cli_args/interactive_init/evm_prompts.rs (2)
224-248
: Clarify the TODO comment about network iteration.The TODO comment suggests uncertainty about whether to use
NetworkWithExplorer::iter()
,HypersyncNetwork::iter()
, or both. This needs clarification to ensure the correct networks are being presented to users.Based on the context,
NetworkWithExplorer::iter()
seems appropriate since this function is specifically for explorer-based imports, but the team should confirm this is the intended behavior.The simplified network selection is much cleaner than the previous implementation, removing the manual ID entry complexity.
310-362
: Excellent error handling for the new NetworkOrChainId type.The
get_network_with_explorer
method properly handles both variants ofNetworkOrChainId
:
- NetworkName variant: Attempts conversion to
NetworkWithExplorer
with clear error messaging- ChainId variant: Two-step conversion (chain ID → Network → NetworkWithExplorer) with appropriate error handling for unsupported networks
- None case: Simplified prompt using all
NetworkWithExplorer
networksThe error messages are clear and actionable, helping users understand why their selection failed.
I made this change over two months ago and they never got around to cleaning it up so here it is. Still some more cleanup to do, but it is a good start.
Screencast.from.2025-06-19.09-07-09.webm
Summary by CodeRabbit
New Features
Improvements