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

Add CBOR pretty print and validation commands to cardano-cli #545

Merged
merged 3 commits into from
Mar 2, 2020

Conversation

Jimbo4350
Copy link
Contributor

@Jimbo4350 Jimbo4350 commented Feb 6, 2020

Issue

checklist

  • This PR contains all the work required to resolve the linked issue.

  • This PR results in breaking changes to upstream dependencies.

  • The work contained has sufficient documentation to describe what it does and how to do it.

  • The work has sufficient tests and/or testing.

  • I have committed clear and descriptive commits. Be considerate as somebody else will have to read these.

  • I have added the appropriate labels to this PR.

@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch 2 times, most recently from 0789dfc to df16374 Compare February 11, 2020 15:52
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch 5 times, most recently from 70c17e2 to d4333b6 Compare February 18, 2020 09:30
@Jimbo4350 Jimbo4350 marked this pull request as ready for review February 18, 2020 09:31
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch from d4333b6 to 305931a Compare February 18, 2020 09:46
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch from 305931a to e8fc296 Compare February 18, 2020 15:42
@Jimbo4350 Jimbo4350 added maintenance Non-critical improvement or maintenance tasks. priority low Issues/RPs that are low priority issues/PRs in relation to a minimum Shelley testnet and Shelley mai labels Feb 18, 2020
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch 3 times, most recently from d049daa to 2c81333 Compare February 18, 2020 16:50
iohk-bors bot added a commit to IntersectMBO/cardano-prelude that referenced this pull request Feb 18, 2020
97: Update `cborg` dependency to include pretty printing fix r=Jimbo4350 a=Jimbo4350

Required for IntersectMBO/cardano-node#545
Pretty printing fix: well-typed/cborg#223

Co-authored-by: Jordan Millar <[email protected]>
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch from 2c81333 to 675ffc2 Compare February 19, 2020 09:50
@Jimbo4350 Jimbo4350 requested review from deepfire and erikd February 19, 2020 09:55
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

This looks ok, but I think we should consider the following:

What we're doing in this patch is adding two intertwined features: validating the parsing of a byron block or tx, and pretty-printing a CBOR file. These things can be separate. We can parse any syntactically valid CBOR file completely generically (as a CBOR Term) without needing to know if it is a tx or a block or a CBOR representation of a cat.

So perhaps we should split this into a command to read and print any CBOR file, and separatly a command to validate a byron tx, or a byron block etc.

@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch 2 times, most recently from 66b2e5b to 70a41c5 Compare February 25, 2020 09:35
@Jimbo4350 Jimbo4350 changed the title Add CBOR pretty print command to cardano-cli Add CBOR pretty print and validation commands to cardano-cli Feb 25, 2020
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch from 70a41c5 to 4aa2a93 Compare February 25, 2020 09:43
@Jimbo4350 Jimbo4350 removed the maintenance Non-critical improvement or maintenance tasks. label Feb 25, 2020
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch from 4aa2a93 to 4c8f8f3 Compare February 25, 2020 13:02
@Jimbo4350 Jimbo4350 added the enhancement New feature or request label Feb 25, 2020
@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 25, 2020

Merge conflict

@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch 3 times, most recently from 2f77d86 to 4bace2d Compare February 28, 2020 09:53
@Jimbo4350 Jimbo4350 force-pushed the jordan/cbor-command-cli branch from 4bace2d to 40d6974 Compare March 2, 2020 09:50
@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 2, 2020

@iohk-bors iohk-bors bot merged commit c34fb9f into master Mar 2, 2020
@iohk-bors iohk-bors bot deleted the jordan/cbor-command-cli branch March 2, 2020 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority low Issues/RPs that are low priority issues/PRs in relation to a minimum Shelley testnet and Shelley mai
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants