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

cardano-cli refactoring & submit-tx subcommand #135

Merged
merged 5 commits into from
Aug 23, 2019
Merged

cardano-cli refactoring & submit-tx subcommand #135

merged 5 commits into from
Aug 23, 2019

Conversation

deepfire
Copy link
Contributor

@deepfire deepfire commented Aug 21, 2019 •
edited

  1. New submit-tx subcommand.
  2. Drop SystemVersion in favor of Protocol.
  3. Move some command parsers to Cardano.Node.Parsers.
  4. Rename and segregate CLIOps (ex-KeyMaterialOps)
  5. Add ByronLegacy to the Protocol type
  6. Remove submit subcommand from cardano-node
  7. Factor orphans.
  8. Factor mkConfiguration
  9. Eliminate all warnings.
  10. Documentation & updated the scripts/submit-tx.sh script.

TODO

  • Separate the first megacommit ?
  • Testing -- this requires a way to create transactions. WIP

@deepfire deepfire mentioned this pull request Aug 21, 2019
Closed
4 tasks
@mrBliss
Copy link
Contributor

mrBliss commented Aug 22, 2019

Just curious (I haven't read through the whole PR yet), what's ByronLegacy going to be?

@deepfire
Copy link
Contributor Author

deepfire commented Aug 22, 2019 •
edited

@mrBliss, the cardano-cli has to deal with legacy Byron formats, so we need to represent those somehow, conceptually. We used to have the separate SystemVersion type for this in cardano-cli, but now that cardano-cli is using more of the shared code, it makes less sense to maintain the extra type.

@deepfire deepfire mentioned this pull request Aug 22, 2019
Closed
@deepfire deepfire marked this pull request as ready for review August 22, 2019 11:09
Jimbo4350
Jimbo4350 requested changes Aug 22, 2019
View reviewed changes
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Good work, however some things need to be put back where they were. See comments.

cardano-node/src/Cardano/Node/TxSubmission.hs Outdated Show resolved Hide resolved
cardano-node/app/cardano-cli.hs Outdated Show resolved Hide resolved
cardano-node/app/cardano-cli.hs Show resolved Hide resolved
cardano-node/src/Cardano/Node/CLI.hs Outdated Show resolved Hide resolved
@deepfire deepfire force-pushed the serge/tx-submission branch 3 times, most recently from 1d5d998 to a8585f9 Compare August 23, 2019 14:08
Jimbo4350
Jimbo4350 requested changes Aug 23, 2019
View reviewed changes
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Btw, great work with removing the warnings! Just a couple things still need to be addressed.

cardano-node/app/cardano-cli.hs Outdated Show resolved Hide resolved
cardano-node/app/cardano-node.hs Show resolved Hide resolved
cardano-node/app/cardano-node.hs Show resolved Hide resolved
@deepfire
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 23, 2019
@iohk-bors @deepfire
Merge #135
1065b2e 
135: cardano-cli refactoring & submit-tx subcommand r=deepfire a=deepfire

0. New `submit-tx` subcommand. 
0. Drop `SystemVersion` in favor of `Protocol`.
0. Move some command parsers to `Cardano.Node.Parsers`.
0. Rename and segregate `CLIOps` (ex-`KeyMaterialOps`)
0. Add `ByronLegacy` to the `Protocol` type
0. Remove `submit` subcommand from `cardano-node`
0. Factor orphans.
0. Factor `mkConfiguration`
0. Eliminate all warnings.
0. Documentation & updated the `scripts/submit-tx.sh` script.

# TODO

- [ ] Separate the first megacommit ?
- [ ] Testing -- this requires a way to create transactions. WIP

Co-authored-by: Kosyrev Serge <serge.kosyrev@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 23, 2019

Build succeeded

@iohk-bors iohk-bors bot merged commit d7af284 into master Aug 23, 2019
@iohk-bors iohk-bors bot deleted the serge/tx-submission branch August 23, 2019 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jimbo4350 Jimbo4350 Jimbo4350 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@deepfire @mrBliss @Jimbo4350