-
Notifications
You must be signed in to change notification settings - Fork 16
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
action create-hardfork cmd + update description fields in all governance action files. #746
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice job! Can you add a test for that, similarly to the other ones here: https://github.com/IntersectMBO/cardano-cli/blob/main/cardano-cli/test/cardano-cli-golden/Test/Golden/Governance/Action.hs
The windows CI failure: IntersectMBO/cardano-node#5811 (comment) - we can ignore windows GHA, it's fixed in nix already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🍾 A few typos in the help needs fixing.
cardano-cli/src/Cardano/CLI/EraBased/Options/Governance/Actions.hs
Outdated
Show resolved
Hide resolved
cardano-cli/src/Cardano/CLI/EraBased/Options/Governance/Actions.hs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Feel free to merge after @smelc approves.
70d5e87
to
e3dc0a0
Compare
82cc5c1
to
b28cb9c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit history needs to be cleaned up. Squashing commits and leaving a commit message like:
Squashed commit of the following:
commit https://github.com/IntersectMBO/cardano-cli/commit/82cc5c1ab7ff50d4072a4e4dea0ba9f6665e04be
Merge: https://github.com/IntersectMBO/cardano-cli/commit/8229fc0c0e3d07f26f1c0707893cf5544746e115 https://github.com/IntersectMBO/cardano-cli/commit/e3dc0a02ffdcd44779a7a7dac99d68d1cc235bdc
Author: Carlos LopezDeLara <[email protected]>
Date: Thu May 2 13:10:52 2024 -0600
Merge branch 'cl/hardforkinit' of github.com:IntersectMBO/cardano-cli into cl/hardforkinit
commit https://github.com/IntersectMBO/cardano-cli/commit/8229fc0c0e3d07f26f1c0707893cf5544746e115
Author: Carlos LopezDeLara <[email protected]>
Date: Wed May 1 12:09:49 2024 -0600
action create-hardfork command
Co-Authored-By: Clément Hurlin <[email protected]>
commit https://github.com/IntersectMBO/cardano-cli/commit/e3dc0a02ffdcd44779a7a7dac99d68d1cc235bdc
Author: Carlos LopezDeLara <[email protected]>
Date: Wed May 1 12:09:49 2024 -0600
action create-hardfork command
Co-Authored-By: Clément Hurlin <[email protected]>
@CarlosLopezDeLara
is not very readable. When you squash commits you need to summarize the changes in those commits in a concise and clear manner.
I also don't know what is happening in 3d8a21f4792c0983b94f493ee064a3203a1c86da
.
@CarlosLopezDeLara> those commits should not be here: Since they have been merged already. I can help you clean this up during our pair programming session 👍 |
6d58209
to
18222b2
Compare
- Implements action create-hardfork command - Update golden files - Include a description on the "description field" of all governance action commands - Export a friendly function that returns a ByteString (so not in IO) - Export buildShelleyAddress
18222b2
to
af0677c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Changelog
Context
Closes #721
How to trust this PR
Run the command
Explore the text envelope en check it with cbor.me
or with
Checklist