Skip to content
This repository was archived by the owner on Dec 13, 2023. It is now read-only.

Wrap existing haskell cli#8

Merged
bgins merged 16 commits intomainfrom
wrap-haskell-cli
Nov 9, 2022
Merged

Wrap existing haskell cli#8
bgins merged 16 commits intomainfrom
wrap-haskell-cli

Conversation

@bgins
Copy link
Copy Markdown
Contributor

@bgins bgins commented Nov 8, 2022

Description

This PR implements initial CLI functionality by wrapping the existing Haskell CLI binary. Following this PR, we will implement each command natively in Rust. 🦀

This PR implements the the following commands:

  • fission setup
  • fission user whoami (and a fission whoami alias)
  • fission user login
  • fission app register
  • fission app info
  • fission app publish
  • fission app delegate

It also implements the following global flags:

  • --verbose
  • --remote
  • --help (descriptions in attributes per command)

We also started to add a src/utils.rs module, but it is not used in the wrapped CLI implementation.

Link to issue

Closes #6

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change that adds functionality)
  • Comments have been added

Testing

Testing will be a bit manual, going through each command to see that it works. For each command, try some of the args and flags to see they work.

The recommended sequence is:

  • Create a new user with cargo run setup
  • Check the user is set up with cargo run user whoami and cargo run whoami
  • Link the user to a browser with cargo run user login
  • Register an app with <path-to-binary> app register
  • Publish the app with <path-to-binary> app publish
  • Get info on the app with <path-to-binary> app info
  • Delegate access to the app with <path-to-binary> app delegate

The last app commands are best tested outside of the project directory, which is why<path-to-binary> is suggested for them. A temp directory with a text file can be created as a simple app to publish.

The app delegate command is slightly more involved than the other commands. At minimum, an app name and an audience must be specified. The app that was created and published while testing app register and app publish can be used for app name. The DID can be any valid did:key. For example:

<path-to-binary> app delegate -a short-old-tin-hero -d did:key:z6Mko4wBW851vmaB2VoA3dR6fisKK3Wn5bEWdhwnCw4yrRAm

The global verbose and remote commands can also be tested. Verbose is a bit easier and can be added along the way. Testing remote is essentially a separate pass through each of the commands.

Comment thread src/cmd/app.rs
Command::new("fission")
.args(["app", "delegate"])
.args(args)
.args(remote)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The remote arg must come after the other args, but before flags for compatibility with the existing CLI.

Comment thread src/cmd/user.rs
pub struct User {
#[clap(subcommand)]
command: UserCommands,
pub command: UserCommands,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Needed to make this public for the whoami command alias here

command: UserCommands::Whoami { verbose, remote },

Would be open to alternative approaches!

@bgins bgins changed the title Wrap haskell cli Wrap existing haskell cli Nov 8, 2022
@bgins bgins marked this pull request as ready for review November 8, 2022 19:28
@bgins bgins requested a review from BoisterousCoder November 8, 2022 19:28
@bgins bgins self-assigned this Nov 8, 2022
@bgins bgins added the enhancement New feature or request label Nov 8, 2022
@BoisterousCoder
Copy link
Copy Markdown
Contributor

BoisterousCoder commented Nov 8, 2022

Everything looks good to me. Though most of the aliases such as fission up are not covered. We might want to make a note somewhere to cover in the future

@bgins
Copy link
Copy Markdown
Contributor Author

bgins commented Nov 9, 2022

Everything looks good to me. Though most of the aliases such as fission up are not covered. We might want to make a note somewhere to cover in the future

Ah yes, thanks for calling that out. Meant to mention it in the PR.

My proposal is that we drop support for the fission up and fission login aliases. fission up primarily exists as a historical artifact because it was implemented before fission app publish in the Haskell CLI. fission login is infrequently used, and I think we can drop it as an alias.

@bgins bgins merged commit 226aca7 into main Nov 9, 2022
@bgins bgins deleted the wrap-haskell-cli branch November 9, 2022 00:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrap existing CLI

2 participants