Skip to content

Added FromJSON instance for PParamUpdates#5281

Merged
Soupstraw merged 1 commit intomasterfrom
jj/ppupdate-json
Sep 11, 2025
Merged

Added FromJSON instance for PParamUpdates#5281
Soupstraw merged 1 commit intomasterfrom
jj/ppupdate-json

Conversation

@Soupstraw
Copy link
Contributor

@Soupstraw Soupstraw commented Sep 9, 2025

Description

This PR adds a FromJSON instance to PParamsUpdate.

close #5052

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@Soupstraw Soupstraw force-pushed the jj/ppupdate-json branch 3 times, most recently from ee15bb5 to a9acf52 Compare September 9, 2025 13:37
@Soupstraw Soupstraw marked this pull request as ready for review September 9, 2025 13:38
@Soupstraw Soupstraw requested a review from a team as a code owner September 9, 2025 13:38
Copy link
Contributor

@aniketd aniketd left a comment

Choose a reason for hiding this comment

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

Nice!

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.

The implementation looks correct to me, but I wonder if my suggestion could prevent the new helper type.
And, if that's not the case, it doesn't seem like we should exporting it.

@Soupstraw Soupstraw force-pushed the jj/ppupdate-json branch 6 times, most recently from b431d8e to fbff13e Compare September 10, 2025 12:20
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.

Looks good to me
This allows us to make the golden tests better, so that the golden files don't get spontaneously regenerated (since they're created with quickcheck).

@teodanciu
Copy link
Contributor

Oh, as we had the release, I think the version of cardano-ledger-core should now change to 1.18.1.0 , and also the changelog entry should be placed accordingly - probably better to merge it after @lehins PR: #5286

@Soupstraw Soupstraw enabled auto-merge (rebase) September 11, 2025 09:11
@Soupstraw Soupstraw merged commit c6074f7 into master Sep 11, 2025
122 checks passed
@Soupstraw Soupstraw deleted the jj/ppupdate-json branch September 11, 2025 11:55
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.

Implement JSON deserializers for PParamsUpdate

3 participants