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

Make Conway-specific queries only available in Conway #4033

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

aniketd
Copy link
Contributor

@aniketd aniketd commented Feb 1, 2024

Description

Resolves #4010

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • When applicable, versions are updated in .cabal and CHANGELOG.md files according to the
    versioning process.
  • The version bounds in .cabal files for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • All visible changes are prepended to the latest section of a CHANGELOG.md for the affected packages. New section is never added with the code changes. (See RELEASING.md)
  • Code is formatted with fourmolu (use scripts/fourmolize.sh)
  • Cabal files are formatted (use scripts/cabal-format.sh)
  • hie.yaml has been updated (use scripts/gen-hie.sh)
  • Self-reviewed the diff

@aniketd aniketd force-pushed the aniketd/conway-specific-queries branch from 76f5161 to c631d6f Compare February 1, 2024 08:22
@aniketd aniketd requested review from lehins and Soupstraw February 1, 2024 08:26
@aniketd aniketd force-pushed the aniketd/conway-specific-queries branch from c631d6f to f163fb3 Compare February 1, 2024 08:29
Copy link
Contributor

@teodanciu teodanciu left a comment

Choose a reason for hiding this comment

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

Depending on when you merge this, but to me it doesn't seem like you need any (EDIT:) major version bumps, because the backwards incompatible changes (removing the functions from the typeclasses) are in shelley and can go in the unreleased 1.10 version, which the other packages already depend on. Conway 1.13 which now contains Constitution is also not released yet, so no bump needed there either.
If I understood correctly the slack conversation about versions and typeclass changes, unless any of the packages are exporting these removed functions, they don't need a bump. EDIT: they do need non-breaking bump, sorry for the confusion

@lehins
Copy link
Collaborator

lehins commented Feb 1, 2024

they don't need a bump.

@teodanciu They do need a bump, just not a breaking version bump

@teodanciu
Copy link
Contributor

they don't need a bump.

@teodanciu They do need a bump, just not a breaking version bump

Makes sense if I think about it. Thanks for clarifying

@aniketd aniketd force-pushed the aniketd/conway-specific-queries branch 2 times, most recently from 6baa5dc to 849f6d4 Compare February 2, 2024 13:33
@aniketd aniketd requested review from teodanciu and lehins February 2, 2024 13:33
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

This looks good. Just need to fixup versions and changelogs

eras/shelley/test-suite/cardano-ledger-shelley-test.cabal Outdated Show resolved Hide resolved
eras/shelley/test-suite/CHANGELOG.md Outdated Show resolved Hide resolved
eras/allegra/impl/CHANGELOG.md Outdated Show resolved Hide resolved
eras/alonzo/impl/CHANGELOG.md Outdated Show resolved Hide resolved
eras/babbage/impl/CHANGELOG.md Outdated Show resolved Hide resolved
eras/mary/impl/CHANGELOG.md Outdated Show resolved Hide resolved
eras/shelley-ma/test-suite/CHANGELOG.md Outdated Show resolved Hide resolved
libs/cardano-ledger-api/CHANGELOG.md Outdated Show resolved Hide resolved
libs/cardano-ledger-api/CHANGELOG.md Outdated Show resolved Hide resolved
@aniketd aniketd force-pushed the aniketd/conway-specific-queries branch 2 times, most recently from 8cb4d1e to 7b05234 Compare February 3, 2024 16:53
@aniketd aniketd requested a review from lehins February 3, 2024 16:53
@aniketd aniketd force-pushed the aniketd/conway-specific-queries branch from 7b05234 to 8267b8c Compare February 5, 2024 19:22
@aniketd aniketd force-pushed the aniketd/conway-specific-queries branch from 8267b8c to 84aa035 Compare February 6, 2024 12:28
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you.

@lehins lehins enabled auto-merge February 6, 2024 12:43
@lehins lehins merged commit 5d6ceb7 into master Feb 6, 2024
11 of 20 checks passed
@neilmayhew neilmayhew deleted the aniketd/conway-specific-queries branch March 8, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Conway specific queries only available in Conway
3 participants