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

Remove epochId argument from getNetworkParameters. #1694

Merged

Conversation

jonathanknowles
Copy link
Contributor

@jonathanknowles jonathanknowles commented May 27, 2020

Issue Number

#1690

Overview

This PR:

  • Removes the epochId argument from the getNetworkParameters API function.
  • Removes the epoch argument from the network parameters CLI command.
  • Removes the now unused ApiEpochNumber type. (We can reinstate it later, if necessary.)
  • Removes the now unused ErrNoSuchEpoch type. (We can reinstate it later, if necessary.)
  • Updates the swagger specification.
  • Updates the CLI integration tests.
  • Updates the API integration tests.

Comments

⚠️ This PR breaks API compatibility with older versions! ⚠️

@jonathanknowles jonathanknowles self-assigned this May 27, 2020
We can revert this change if we ever need to reinstate the type.
We no longer need to test the `epochId` path parameter for
the `getNetworkParameters` API function.
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/remove-epochId-from-getNetworkParameters branch from 5b9c039 to 873f0eb Compare May 27, 2020 05:10
We no longer need to test the `epoch` parameter for the
`network parameters` function.
We no longer need the `epochId` path parameter for
the `getNetworkParameters` function.
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/remove-epochId-from-getNetworkParameters branch from 56a49af to c127bea Compare May 27, 2020 05:17
@jonathanknowles jonathanknowles changed the title WIP: Remove epochId argument from getNetworkParameters. Remove epochId argument from getNetworkParameters. May 27, 2020
We no longer expect to see the `EPOCH_NUMBER` parameter in the CLI help
for `network parameters`.
@jonathanknowles jonathanknowles requested review from piotr-iohk and rvl May 27, 2020 06:35
@jonathanknowles jonathanknowles marked this pull request as ready for review May 27, 2020 06:35
Copy link
Contributor

@piotr-iohk piotr-iohk left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

👍

lib/cli/test/unit/Cardano/CLISpec.hs Show resolved Hide resolved
lib/core/src/Cardano/Wallet/Api/Server.hs Show resolved Hide resolved
@jonathanknowles jonathanknowles merged commit c01e718 into master May 27, 2020
@jonathanknowles jonathanknowles deleted the jonathanknowles/remove-epochId-from-getNetworkParameters branch May 27, 2020 10:28
iohk-bors bot added a commit that referenced this pull request May 28, 2020
1698: Clarify the description of `getNetworkParameters` in the API and CLI. r=jonathanknowles a=jonathanknowles

# Related Issues

#1690

# Overview

This PR clarifies the descriptions of the `getNetworkParameters` API endpoint and the associated `network parameters` CLI command, in response to [review feedback](https://github.com/input-output-hk/cardano-wallet/pull/1694/files#r431011565) for #1694, 

Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot added a commit that referenced this pull request May 28, 2020
1698: Clarify the description of `getNetworkParameters` in the API and CLI. r=jonathanknowles a=jonathanknowles

# Related Issues

#1690

# Overview

This PR clarifies the descriptions of the `getNetworkParameters` API endpoint and the associated `network parameters` CLI command, in response to [review feedback](https://github.com/input-output-hk/cardano-wallet/pull/1694/files#r431011565) for #1694, 

Co-authored-by: Jonathan Knowles <[email protected]>
@jonathanknowles jonathanknowles added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants