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

feat: utilize ffcli (CLI refactor) #497

Merged
merged 50 commits into from
Mar 1, 2023
Merged

feat: utilize ffcli (CLI refactor) #497

merged 50 commits into from
Mar 1, 2023

Conversation

zivkovicmilos
Copy link
Member

Description

This PR introduces the ffcli package, and reworks how existing commands are structured so they can utilize it.
The functionality of the commands themselves is unchanged, however, their help and error outputs are vastly improved.

Based on discussions from #460, the ffcli package is wrapped within pkgs/commands.

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist (for contributors)

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Manual tests

Manually executed changed commands.

Additional comments

cc @moul @harry-hov

@zivkovicmilos zivkovicmilos added the 🌱 feature New update to Gno label Jan 31, 2023
@zivkovicmilos zivkovicmilos self-assigned this Jan 31, 2023
@zivkovicmilos zivkovicmilos mentioned this pull request Jan 31, 2023
Closed
moul
moul reviewed Jan 31, 2023
View reviewed changes
cmd/gnoland/main.go Outdated Show resolved Hide resolved
moul
moul reviewed Jan 31, 2023
View reviewed changes
cmd/gnoland/main.go Outdated Show resolved Hide resolved
moul
moul reviewed Jan 31, 2023
View reviewed changes
pkgs/commands/command.go Outdated Show resolved Hide resolved
moul
moul reviewed Jan 31, 2023
View reviewed changes
cmd/gnotxport/export.go Outdated Show resolved Hide resolved
@zivkovicmilos
Copy link
Member Author

zivkovicmilos commented Feb 8, 2023 •
edited
Loading

@moul

I've slightly tweaked the API for the pkgs/Command initialization - the configurations are no longer expected to implement the exec method, but the user can actually supply any kind of exec method they want (that can take in the configuration for example).

I've also thought about completely ditching this Command wrapper and using ffcli methods directly, but I think it's easier for the user to not have to manage the flagset, especially for subcommands (they would need to specifically register the flags for each command, and to register the parent command's flagset as well - the Command wrapper already does this for them behind the scenes).

Here is the commit hash that has the changes we discussed:
962860d

@zivkovicmilos
Copy link
Member Author

@moul
Does this API change I've mentioned work for you, so I can start reworking other commands as well?

@moul
Copy link
Member

moul commented Feb 11, 2023 •
edited
Loading

Hey, yes, it’s a way better, let’s give a more complete try. Thank you.

@zivkovicmilos zivkovicmilos marked this pull request as ready for review February 20, 2023 13:18
@zivkovicmilos zivkovicmilos requested a review from a team as a code owner February 20, 2023 13:18
moul
moul reviewed Feb 21, 2023
View reviewed changes
cmd/common/common.go Outdated Show resolved Hide resolved
moul
moul reviewed Feb 21, 2023
View reviewed changes
cmd/genproto/genproto.go Outdated Show resolved Hide resolved
moul
moul reviewed Feb 21, 2023
View reviewed changes
cmd/gnokey/add.go Outdated Show resolved Hide resolved
moul
moul requested changes Feb 21, 2023
View reviewed changes
Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

I made a new round of comments but it’s not exhaustive, I’ll make a new review later.

Btw, I like the new commands API like this, well done 👍.

@zivkovicmilos
Copy link
Member Author

@moul

Merged out changes from master

@moul moul merged commit eabc5c4 into gnolang:master Mar 1, 2023
@zivkovicmilos zivkovicmilos deleted the feature/improve-cli branch March 1, 2023 13:54
@tadvi tadvi mentioned this pull request Mar 3, 2023
Closed
harry-hov pushed a commit to harry-hov/gno that referenced this pull request Mar 6, 2023
@zivkovicmilos @harry-hov
feat: utilize ffcli (CLI refactor) (gnolang#497)
7101baa
harry-hov pushed a commit to harry-hov/gno that referenced this pull request Mar 7, 2023
@zivkovicmilos @harry-hov
feat: utilize ffcli (CLI refactor) (gnolang#497)
3e44da4
grepsuzette added a commit to grepsuzette/gnoland-tutorials that referenced this pull request Mar 14, 2023
@grepsuzette
Change gnodev, gnokey and gnofaucets commands so they are compatible …
32e6dc3 
…the new cli

Since the new cli gnolang/gno#497 2 weeks ago,
the tools like gnodev use single dash options (-foo) instead of --foo,
and the address has to be put at the end. The pictures aren't changed
however.
@grepsuzette grepsuzette mentioned this pull request Mar 14, 2023
Open
@r3v4s r3v4s mentioned this pull request Mar 23, 2023
Merged
@moul moul mentioned this pull request Apr 11, 2023
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moul moul moul approved these changes

Assignees

@zivkovicmilos zivkovicmilos

Labels
🌱 feature New update to Gno
Projects
🚀 The Launch [DEPRECATED]
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zivkovicmilos @moul