Skip to content
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

refactor arguments display #491

Merged
merged 2 commits into from
Sep 1, 2022
Merged

refactor arguments display #491

merged 2 commits into from
Sep 1, 2022

Conversation

nlordell
Copy link
Contributor

@nlordell nlordell commented Sep 1, 2022

This PR refactors arguments::display_* to do a little more than just display the arguments. When implementing additional arguments flags for other PRs, I found the pattern required to use those helper methods quite tedious and error-prone (I would forget the writeln at the end, use writeln instead of write for the name, place another field between the display_* and writeln!(f) to name a few):

write!(f, "some_flag: ")?;
display_X(&f.some_flag, f)?;
writeln!(f)?;

Since the arguments::display_* methods are only ever used for fields, I thought it made sense to specialize them so its a bit more ergonomic to use:

display_X(f, "some_flag", &f.some_flag)?;

Test Plan

Run the crates and see the formatted arguments still work (not included here as it would make the PR description too long).

Some highlights:

Before:

...
balancer_sor_url: price_estimation_rate_limiter: None
...
disabled_one_inch_protocols: ["PMM1"]
...
flashbots_api_url: [https://rpc.flashbots.net/, ]
...
additional_tip_percentage: 0.05
...
solver_account: Some(PrivateKey(PrivateKey(0x1111111111111111111111111111111111111111)))
...
external_solvers: Some([ExternalSolverArg { name: "example", url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("example.com")), port: None, path: "/", query: None, fragment: None }, account: Address(0x1111111111111111111111111111111111111111) }
...

After:

...

balancer_sor_url: None
price_estimation_rate_limiter: None
...
disabled_one_inch_protocols: [PMM1]
...
flashbots_api_url: [https://rpc.flashbots.net/]
...
additional_tip_percentage: 0.05%
...
solver_account: PrivateKey(0x1111111111111111111111111111111111111111)
...
external_solvers: [example|https://example.com/|Address(0x1111111111111111111111111111111111111111)]
...

@nlordell nlordell requested a review from a team as a code owner September 1, 2022 07:46
@nlordell nlordell merged commit a47c41e into main Sep 1, 2022
@nlordell nlordell deleted the refactor-args-display branch September 1, 2022 08:19
@github-actions github-actions bot locked and limited conversation to collaborators Sep 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants