IntersectMBO / cardano-cli

This repository contains sources for the command-line interface (CLI) tool for interacting with the Cardano blockchain.
Apache License 2.0
33 stars 13 forks source link

Move "conway governance hash" commands to "hash" #787

Closed smelc closed 2 weeks ago

smelc commented 1 month ago

Changelog

- description: |
    Move "conway governance hash" commands to "hash". Users should adapt their calls as follows:

    `cardano-cli conway governance hash anchor-data ...` becomes `cardano-cli hash anchor-data ...`
    `cardano-cli conway governance hash script ...` becomes `cardano-cli hash script ...`
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

Fixes https://github.com/IntersectMBO/cardano-cli/issues/782

smelc commented 1 month ago

So what we really want is a top level command that can hash all the hashable things. Or at least not to give the impression that hashing depends on the era.

cc @CarlosLopezDeLara since the current design is your ask from https://github.com/IntersectMBO/cardano-cli/issues/782

I can make hash a top-level command if you approve that like @Jimbo4350 does

CarlosLopezDeLara commented 1 month ago

@smelc I like the idea! sounds great.

smelc commented 3 weeks ago

@Jimbo4350> Made the command top-level and removed the era.

I will consider in a follow-up PR if other hashing commands can be moved there too (hashing keys).

CarlosLopezDeLara commented 2 weeks ago

LGTM! However I would be wary of breaking the existing hashing functionality. Talk to @CarlosLopezDeLara about this as it may be an annoying for users who have existing scripts etc.

I think we can afford the breaking change here since 9.0 is still in the makings and 8.12 will not cross the hardfork. I'll update docs after this is on a cardano-cli release.

gitmachtl commented 2 weeks ago

@smelc there are a lot of leftovers in the help texts like:

...
  --anchor-data-hash HASH  Proposal anchor data hash (obtain it with
                           "cardano-cli conway governance hash anchor-data ...")
  --constitution-url TEXT  Constitution URL.
  --constitution-hash HASH Hash of the constitution data (obtain it with
                           "cardano-cli conway governance hash anchor-data
                           ...").
...

pointing to the old location of the hash function conway governance hash

smelc commented 2 weeks ago

@gitmachtl> yes, it's being fixed by https://github.com/IntersectMBO/cardano-cli/pull/821. I think that's because I I lost some of my changes while polishing the PR :frowning_face: