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.

Sign up for GitHub

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

Jump to bottom

Add immutable tip parameter to conway queries #603

Merged
merged 1 commit into from
Feb 12, 2024
Merged

Add immutable tip parameter to conway queries #603

merged 1 commit into from
Feb 12, 2024

Conversation

palas
Copy link
Contributor

@palas palas commented Feb 9, 2024 •
edited
Loading

Changelog

- description: |
    Allow users to specify the target in conway queries (either immutable tip or volatile tip)
  type:
    - feature        # introduces a new feature

How to trust this PR

Don't trust it, look at it with a magnifying glass.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

Jimbo4350
Jimbo4350 reviewed Feb 9, 2024
View reviewed changes
cardano-cli/src/Cardano/CLI/EraBased/Options/Common.hs Show resolved Hide resolved
cardano-cli/src/Cardano/CLI/EraBased/Options/Query.hs
fmap QueryTipCmd $
QueryTipCmdArgs
<$> pSocketPath envCli
<*> pConsensusModeParams
<*> pNetworkId envCli
<*> pTarget era
Copy link
Contributor

Choose a reason for hiding this comment

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

Then what you can do here is use maybe and forEraMaybeEon so that when we cannot get the ConwayEraOnwards witness, we default to VolatileTip.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't that go inside the pTarget as, in my opinion, the defaulting is its implementation detail?

@palas palas changed the title Add immutable tip parameter to conway tip query Add immutable tip parameter to conway queries Feb 9, 2024
@palas palas added the enhancement New feature or request label Feb 9, 2024
@palas palas self-assigned this Feb 9, 2024
@palas palas marked this pull request as ready for review February 9, 2024 23:52
@palas palas requested a review from Jimbo4350 February 9, 2024 23:52
carbolymer
carbolymer reviewed Feb 12, 2024
View reviewed changes
Copy link
Contributor

@carbolymer carbolymer left a comment

Choose a reason for hiding this comment

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

Nice job!

@palas palas requested a review from carbolymer February 12, 2024 15:49
@palas palas added this pull request to the merge queue Feb 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 12, 2024
@palas palas added this pull request to the merge queue Feb 12, 2024
@palas palas removed this pull request from the merge queue due to a manual request Feb 12, 2024
@palas palas added this pull request to the merge queue Feb 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 12, 2024
@palas palas enabled auto-merge February 12, 2024 17:51
@palas palas added this pull request to the merge queue Feb 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Feb 12, 2024
@palas palas closed this Feb 12, 2024
@palas
Copy link
Contributor Author

palas commented Feb 12, 2024

Closed by accident

@palas palas reopened this Feb 12, 2024
@palas palas enabled auto-merge February 12, 2024 19:45
@palas palas added this pull request to the merge queue Feb 12, 2024
Merged via the queue into IntersectMBO:main with commit 07180c3 Feb 12, 2024
16 checks passed
@palas palas deleted the add-immutable-tip branch February 12, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jimbo4350 Jimbo4350 Jimbo4350 approved these changes

@newhoggy newhoggy Awaiting requested review from newhoggy

@kevinhammond kevinhammond Awaiting requested review from kevinhammond

@disassembler disassembler Awaiting requested review from disassembler

@carbolymer carbolymer Awaiting requested review from carbolymer

Assignees

@palas palas

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@palas @carbolymer @Jimbo4350