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.

Sign up for GitHub

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

Jump to bottom

[refactor] #4167: asset subcommand in client CLI to be consistent #4200

Merged
merged 4 commits into from
Jan 23, 2024
Merged

[refactor] #4167: asset subcommand in client CLI to be consistent #4200

merged 4 commits into from
Jan 23, 2024

Conversation

Asem-Abdelhady
Copy link
Contributor

@Asem-Abdelhady Asem-Abdelhady commented Jan 12, 2024 •
edited
Loading

Description

In asset subcommand in the CLI operations like mint, burn, get, transfer had register. I refactored the subcommand to follow the suggestions in #4167.

Linked issue

Closes #4167

Use cases

Register

./iroha_client_cli asset  --definition-id="tea#looking_glass" --value-type=Quantity

Mint

./iroha_client_cli asset mint --asset-id="tea##mad_hatter@looking_glass" --quantity="100"

Burn

./iroha_client_cli asset burn --asset-id="tea##mad_hatter@looking_glass" --quantity="10"

Get

./iroha_client_cli asset get --asset-id="tea##mad_hatter@looking_glass"

Transfer

./iroha_client_cli asset transfer --to="white_rabbit@looking_glass" --asset-id="tea##mad_hatter@looking_glass" --quantity="5"

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jan 12, 2024
@Asem-Abdelhady Asem-Abdelhady force-pushed the issue-4167 branch 2 times, most recently from b2a22da to 693e27c Compare January 12, 2024 13:34
@Arjentix Arjentix self-assigned this Jan 12, 2024
Arjentix
Arjentix previously approved these changes Jan 12, 2024
View reviewed changes
client_cli/src/main.rs Outdated Show resolved Hide resolved
@Arjentix
Copy link
Contributor

Need to make CI pass fine

@Asem-Abdelhady
Copy link
Contributor Author

Asem-Abdelhady commented Jan 12, 2024 •
edited
Loading

Need to make CI pass fine

Yes, sure for that I would contact Alex when he gets better.

VAmuzing
VAmuzing previously approved these changes Jan 14, 2024
View reviewed changes
Copy link
Contributor

@VAmuzing VAmuzing left a comment

Choose a reason for hiding this comment

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

Seems like the help section for the commands in #4167 should also be updated to reflect removal of 'from' parameters

@Asem-Abdelhady
Copy link
Contributor Author

Seems like the help section for the commands in #4167 should also be updated to reflect removal of 'from' parameters

yes, i will do that

0x009922
0x009922 reviewed Jan 15, 2024
View reviewed changes
client_cli/src/main.rs Outdated Show resolved Hide resolved
client_cli/src/main.rs Outdated Show resolved Hide resolved
@Asem-Abdelhady Asem-Abdelhady dismissed stale reviews from Arjentix and VAmuzing via 2d39411 January 15, 2024 08:59
@coveralls
Copy link

coveralls commented Jan 15, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 7528303053

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.0%) to 57.044%
Totals Coverage Status
Change from base Build 7285255818: 1.0%
Covered Lines: 22617
Relevant Lines: 39648
💛 - Coveralls

@Asem-Abdelhady Asem-Abdelhady changed the title [refactor] #4167: asset subcommand in CLI to be consistent [refactor] #4167: asset subcommand in client CLI to be consistent Jan 16, 2024
@Asem-Abdelhady Asem-Abdelhady force-pushed the issue-4167 branch 2 times, most recently from 3ed6cc8 to 51ffe9b Compare January 18, 2024 19:26
@Asem-Abdelhady Asem-Abdelhady force-pushed the issue-4167 branch 5 times, most recently from 5ddb06b to aee145e Compare January 18, 2024 20:58
@Asem-Abdelhady @AlexStroke
[change]: cli asset parameters to be consistent
45c2a17 
Signed-off-by: Asem-Abdelhady <asemshawkey@gmail.com>
alexstroke and others added 2 commits January 19, 2024 11:15
@AlexStroke
[fix] Update client cli command for tests
c0232ce 
Signed-off-by: alexstroke <111361420+astrokov7@users.noreply.github.com>
@Asem-Abdelhady
[fix] minor fixes in asset subcommands hints
b696240 
Signed-off-by: Asem-Abdelhady <asemshawkey@gmail.com>
@Asem-Abdelhady Asem-Abdelhady merged commit 85d03b7 into hyperledger-iroha:iroha2-dev Jan 23, 2024
10 checks passed
@Asem-Abdelhady Asem-Abdelhady deleted the issue-4167 branch January 23, 2024 07:38
Asem-Abdelhady added a commit to Asem-Abdelhady/iroha that referenced this pull request Feb 11, 2024
@Asem-Abdelhady
[refactor] hyperledger-iroha#4167: asset subcommand in client CLI to …
ec00331 
…be consistent (hyperledger-iroha#4200)

* [change]: cli asset parameters to be consistent

Signed-off-by: Asem-Abdelhady <asemshawkey@gmail.com>

* [fix] Update client cli command for tests

Signed-off-by: alexstroke <111361420+astrokov7@users.noreply.github.com>


---------

Signed-off-by: Asem-Abdelhady <asemshawkey@gmail.com>
Signed-off-by: alexstroke <111361420+astrokov7@users.noreply.github.com>
Co-authored-by: alexstroke <busyfifer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0x009922 0x009922 0x009922 left review comments

@Arjentix Arjentix Arjentix approved these changes

@VAmuzing VAmuzing VAmuzing approved these changes

@s8sato s8sato Awaiting requested review from s8sato

@mversic mversic Awaiting requested review from mversic

@SamHSmith SamHSmith Awaiting requested review from SamHSmith

@QuentinI QuentinI Awaiting requested review from QuentinI

@outoftardis outoftardis Awaiting requested review from outoftardis

@ilchu ilchu Awaiting requested review from ilchu

@pesterev pesterev Awaiting requested review from pesterev

@Erigara Erigara Awaiting requested review from Erigara

@DCNick3 DCNick3 Awaiting requested review from DCNick3

Assignees

@VAmuzing VAmuzing

@Arjentix Arjentix

@Asem-Abdelhady Asem-Abdelhady

Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Asem-Abdelhady @Arjentix @coveralls @VAmuzing @0x009922