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

token-cli: create transfer-fee-basis-points and transfer-fee-maximum-fee flags #7053

Merged
merged 5 commits into from
Aug 1, 2024

Conversation

Hrushi20
Copy link
Contributor

@Hrushi20 Hrushi20 commented Jul 26, 2024

Split transfer-fee into two separate flags i.e transfer-fee-basis-points and transfer-fee-maximum-fee. Add a message to deprecate transfer-fee.

Fixes #7020

@mergify mergify bot added the community Community contribution label Jul 26, 2024
Hrushi20 added 2 commits July 26, 2024 15:11
Signed-off-by: Hrushi20 <[email protected]>

remove unwanted space

Signed-off-by: Hrushi20 <[email protected]>

added missing comma

Signed-off-by: Hrushi20 <[email protected]>
@Hrushi20 Hrushi20 force-pushed the feature-set-transfer-fee branch from 9058afe to b693738 Compare July 26, 2024 09:43
@Hrushi20
Copy link
Contributor Author

Do I add a test for transfer-fee-maximum-fee? Any more specific tests I need to add?

Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, it looks good overall! Mostly some stylistic comments.

Also, can you run ./cargo-nightly fmt --all from the top of the repo? There's going to be some formatting issues when I run CI

token/cli/src/clap_app.rs Show resolved Hide resolved
token/cli/src/clap_app.rs Outdated Show resolved Hide resolved
token/cli/src/clap_app.rs Outdated Show resolved Hide resolved
token/cli/src/clap_app.rs Show resolved Hide resolved
token/cli/src/clap_app.rs Show resolved Hide resolved
token/cli/src/command.rs Outdated Show resolved Hide resolved
token/cli/src/command.rs Outdated Show resolved Hide resolved
token/cli/src/command.rs Outdated Show resolved Hide resolved
token/cli/src/command.rs Outdated Show resolved Hide resolved
token/cli/tests/command.rs Outdated Show resolved Hide resolved
@joncinque joncinque changed the title create transfer-fee-basis-points and transfer-fee-maximum-fee flags token-cli: create transfer-fee-basis-points and transfer-fee-maximum-fee flags Jul 31, 2024
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

It looks like this is failing some of the clippy lints -- can you run ./cargo-nightly clippy -Zunstable-options --all-targets --features test-sbf -- --deny=warnings --deny=clippy::arithmetic_side_effects from the top of the repo and fix whatever's reported?

We're almost there!

token/cli/src/clap_app.rs Show resolved Hide resolved
token/cli/src/clap_app.rs Show resolved Hide resolved
@Hrushi20 Hrushi20 requested a review from joncinque August 1, 2024 19:19
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

thanks

@joncinque joncinque merged commit 8d115f1 into solana-labs:master Aug 1, 2024
31 checks passed
@Hrushi20 Hrushi20 deleted the feature-set-transfer-fee branch August 2, 2024 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

token-cli: set-transfer-fee has different granularity than create-token
2 participants