Plug *-poll legacy commands back #349

smelc · 2023-10-09T13:17:26Z

Changelog

- description: |
    Bring back legacy *-poll commands
# 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
  # - improvement    # QoL changes e.g. refactoring
  # - 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

@newhoggy witnessed after #322 merge that it was removing two many cmomands: the legacy voting commands should have been kept.

This PR brings back the legacy poll commands.

Fix #340

Checklist

Commit sequence broadly makes sense and commits have useful messages
The change log section in the PR description has been filled in
NA New tests are added if needed and existing tests are updated. These may include:
- golden tests
- property tests
- round trip tests
- integration tests
  See Running tests for more details
NA The version bounds in .cabal files are updated
CI passes. See note on CI. The following CI checks are required:
- Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
- Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
- Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
Self-reviewed the diff

cardano-cli/src/Cardano/CLI/EraBased/Commands/Governance.hs

cardano-cli/src/Cardano/CLI/EraBased/Commands/Governance/Poll.hs

cardano-cli/src/Cardano/CLI/EraBased/Options/Governance/Poll.hs

cardano-cli/src/Cardano/CLI/Legacy/Options.hs

cardano-cli/src/Cardano/CLI/Legacy/Run/Governance.hs

cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Poll.hs

newhoggy · 2023-10-10T01:36:22Z

cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Poll.hs


 runGovernanceCreatePoll
-  :: BabbageEraOnwards era


Change this to be BabbageEraOnly

I can't yet, because we need a release of cardano-api to bring the PR introducing BabbageEraOnly. But I will do so when possible 👍

cardano-cli/src/Cardano/CLI/Legacy/Run/Governance.hs

newhoggy · 2023-10-10T12:06:10Z

cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Poll.hs

+  ( runGovernancePollCmds,
+    runGovernanceCreatePoll, {- For legacy caller only -}
+    runGovernanceAnswerPoll, {- For legacy caller only -}
+    runGovernanceVerifyPoll  {- For legacy caller only -}


We don't need to add a comment here. It is okay to export run commands even if no-one uses them.

In fact, it is preferred in case we want to change the tests to use these instead of the CLI.

Changed 👍

newhoggy · 2023-10-10T12:07:05Z

cardano-cli/src/Cardano/CLI/Legacy/Commands/Governance.hs

+  | GovernanceVerifyPoll
+      (File GovernancePoll In) -- Poll file
+      (File (Tx ()) In) -- Tx file
+      (Maybe (File () Out)) -- Tx file


Comments are good. If used, then use -- ^.

Good catch 👍 Changed.

newhoggy · 2023-10-10T12:10:51Z

cardano-cli/src/Cardano/CLI/Legacy/Run/Governance.hs

+    {- TODO replace BabbageEraOnwardsBabbage by BabbageEraOnlyBabbage when available -}
+    runGovernanceAnswerPoll BabbageEraOnwardsBabbage poll ix mOutFile
+  GovernanceVerifyPoll poll metadata mOutFile ->
+    {- TODO replace BabbageEraOnwardsBabbage by BabbageEraOnlyBabbage when available -}


We shouldn't have to add these comments because when the run command's eon witness type is changed we will get a type error here and know to change it.

In any case, I originally asked for this to be changed to BabbageEraOnly. I now think we should keep the eon type as it is. When we hard fork to Conway, users of this command should still expect this to continue working.

We can deprecate it post-Conway hard-fork.

Removed the comments 👍

You're right, I add in mind those poll commands where babbage only, but this is the legacy part we're speaking here; so it doesn't apply.

smelc force-pushed the smelc/replug-poll-legacy-commands branch 3 times, most recently from e2a4f8a to 2810fa1 Compare October 9, 2023 13:47