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

Merge cardano-wallet-launcher into cardano-wallet #368

Closed
4 tasks done
KtorZ opened this issue Jun 5, 2019 · 2 comments
Closed
4 tasks done

Merge cardano-wallet-launcher into cardano-wallet #368

KtorZ opened this issue Jun 5, 2019 · 2 comments
Assignees

Comments

@KtorZ
Copy link
Member

KtorZ commented Jun 5, 2019

Context

We currently maintain two separate executables as we historically needed to have a separate
process for launching and monitoring both the wallet server and its backend node. It was recently agreed with Daedalus' team that in the long run, Daedalus will be managing both processes itself and would only require the wallet to take care of itself.

Decision

  • Moving towards this direction, we agreed on making the launcher a command of the cardano-wallet CLI instead of a new executable. This will make the support of multiple target backend slightly easier (having to maintain ultimately 3 executables instead of 6, and having the ability to simply remove the launcher command once Daedalus takes care of these bits).

  • Friendly reminder to also update the .travis.yml file to make sure we adjust the build artifacts on releases.

Acceptance Criteria

  1. cardano-wallet must have a new command launcher doing what cardano-wallet-launcher currently does
  2. cardano-wallet-launcher mustn't exist anymore

Development Plan

PR

Number Base
#414 master
#422 master

QA

  • There's no more cardano-wallet-launcher but a new launch command on cardano-wallet. Various tests and documentation bits have been updated to reflect that new setup.

  • There was some issues with the --port validation also fixed as part of this ticket (with corresponding tests at both the cli-level and the unit level).

@jonathanknowles
Copy link
Member

@KtorZ

cardano-wallet must have a new command launcher doing what cardano-wallet-launcher currently does

If commands are verbs, the perhaps we should call this command cardano-wallet launch?

@piotr-iohk
Copy link
Contributor

lgtm

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

No branches or pull requests

3 participants