-
Notifications
You must be signed in to change notification settings - Fork 371
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
feat: add CLI commands for RPC URL and block number #1194
Conversation
WalkthroughThe latest update enhances the application by integrating new command-line options for specifying RPC URLs and block numbers. This enhancement not only broadens the configurability through the command line but also ensures that these settings can default to environment variables if not explicitly provided, offering a flexible and user-friendly approach to configuration management. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Main.hs (4 hunks)
Additional comments: 4
src/Main.hs (4)
- 19-19: The addition of
Word64
to the import list fromData.Word
is necessary for supporting the new--rpc-block
CLI argument, which expects a block number of typeWord64
. This change is appropriate and ensures that the necessary types are available for handling the new functionality.- 136-137: The addition of
cliRpcUrl
andcliRpcBlock
to theOptions
data type introduces new fields to store the values of the--rpc-url
and--rpc-block
CLI arguments, respectively. This change is crucial for enabling the new functionality and allows these values to be passed through the application. The choice ofMaybe Text
forcliRpcUrl
andMaybe Word64
forcliRpcBlock
is appropriate, as it allows for the absence of these arguments, aligning with the described precedence of configuration settings.- 197-202: The update to the
options
parser function to handle the--rpc-url
and--rpc-block
arguments is implemented correctly. The use ofoptional
and the appropriate types (str
for--rpc-url
andauto
for--rpc-block
) ensures that these arguments are optional and parsed correctly. The help messages provided are clear and informative, guiding the user on how to use these new options.- 217-218: The modifications to the
overrideConfig
function to prioritize CLI-provided values forrpcUrl
andrpcBlock
over environment variables and default configurations are implemented as described. The use of the<|>
operator to select the first non-Nothing
value among the CLI argument, environment variable, and existing configuration ensures that the precedence order is respected. This change effectively integrates the new CLI arguments into the application's configuration logic.
63cb249
to
61606d5
Compare
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Main.hs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Main.hs
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Main.hs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Main.hs
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.
Thanks for contributing! There is a little mistake in your solution but it's easily fixable.
src/Main.hs
Outdated
@@ -206,8 +214,8 @@ versionOption = infoOption | |||
|
|||
overrideConfig :: EConfig -> Options -> IO EConfig | |||
overrideConfig config Options{..} = do | |||
rpcUrl <- Onchain.rpcUrlEnv | |||
rpcBlock <- Onchain.rpcBlockEnv | |||
rpcUrl <- (pure cliRpcUrl) <|> Onchain.rpcUrlEnv |
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.
This unfortunately won't work since <|> here operates on IO, not on Maybe. Can you rename rpcUrl
to envRpcUrl
(same for block) and change this instead: rpcUrl = cliRpcUrl <|> envRpcUrl <|> config.rpcUrl
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.
Updated. Thanks for the quick review!
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Main.hs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Main.hs
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.
LGTM!
Implemented CLI arguments
--rpc-url
and--rpc-block
to allow overriding the corresponding environment variables.The precedence for settings is now: Command-Line Arguments > Environment Variables > Configuration Defaults.
Closes #1193
Summary by CodeRabbit