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

clap-utils: Add more compute unit helpers #440

Merged
merged 5 commits into from
Mar 28, 2024

Conversation

joncinque
Copy link

Problem

While working on adding compute budget instructions to CLIs, I've noticed some bits missing in the clap-utils:

  • There's no arg for setting the compute unit limit
  • There's no support for clap v3

Summary of Changes

Pretty simple: add a compute unit limit arg to clap-utils, and both args to clap-v3-utils.

While working on this, it made sense to move the current compute_unit_price helper into a more general compute_budget module. Let me know how this looks!

samkim-crypto
samkim-crypto previously approved these changes Mar 27, 2024
Copy link

@samkim-crypto samkim-crypto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me!

.value_name("COMPUTE-UNIT-PRICE")
.help(COMPUTE_UNIT_PRICE_ARG.help)
}
pub use crate::compute_budget::{compute_unit_price_arg, COMPUTE_UNIT_PRICE_ARG};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion could be to deprecate the re-export with a message to use them directly from compute_budget instead as done here. However, this doesn't really do anything in terms of triggering warnings (solana-labs#33276 (comment)), so it is your call.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that was my thinking too, but I guess it can't hurt to document the code. Done!

@joncinque joncinque merged commit 8246590 into anza-xyz:master Mar 28, 2024
37 checks passed
@joncinque joncinque deleted the culimit branch March 28, 2024 00:58
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