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 lenses to PParams, create a [Era]EraPParams class for each era that needs it #3045

Closed
wants to merge 5 commits into from
Closed

Add lenses to PParams, create a [Era]EraPParams class for each era that needs it #3045

wants to merge 5 commits into from

Conversation

Soupstraw
Copy link
Contributor

@Soupstraw Soupstraw commented Sep 20, 2022 •
edited
Loading

closes #2927

@Soupstraw Soupstraw changed the title WIP: Jj/pparams lens WIP: Add lenses to PParams, create a [Era]EraPParams class for each era that needs it Sep 20, 2022
@Soupstraw Soupstraw force-pushed the jj/pparams-lens branch 2 times, most recently from fe20513 to 357152a Compare September 22, 2022 10:02
@Soupstraw Soupstraw force-pushed the jj/pparams-lens branch 2 times, most recently from e899a34 to 4c28371 Compare October 3, 2022 09:51
@aniketd aniketd mentioned this pull request Oct 5, 2022
Closed
15 tasks
@Soupstraw Soupstraw force-pushed the jj/pparams-lens branch 2 times, most recently from 65a334e to ae70f7f Compare October 10, 2022 16:23
@aniketd aniketd mentioned this pull request Oct 12, 2022
Closed
@aniketd aniketd force-pushed the jj/pparams-lens branch 2 times, most recently from 4a09653 to 2ee6e84 Compare October 21, 2022 13:59
lehins
lehins reviewed Oct 26, 2022
View reviewed changes
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.

I thought I'll skim through the PR. Here are some comments and questions from me

eras/shelley/impl/src/Cardano/Ledger/Shelley/HardForks.hs Show resolved Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/PParams.hs Show resolved Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/Utils.hs Show resolved Hide resolved
eras/shelley/test-suite/bench/Cardano/Ledger/Shelley/Bench/Gen.hs Show resolved Hide resolved
eras/shelley/test-suite/bench/Cardano/Ledger/Shelley/Bench/Gen.hs Show resolved Hide resolved
eras/shelley/test-suite/bench/Main.hs Show resolved Hide resolved
eras/shelley/test-suite/bench/Main.hs Show resolved Hide resolved
@Soupstraw Soupstraw mentioned this pull request Nov 2, 2022
Open
@Soupstraw
Copy link
Contributor Author

This PR has become stale

@Soupstraw Soupstraw closed this Nov 30, 2022
@lehins lehins reopened this Nov 30, 2022
@lehins
Copy link
Collaborator

lehins commented Nov 30, 2022

@Soupstraw Despite that this PR has diverged a bit from master it is still useful work that will be picked up a bit later. Currently it is blocked by #3171

@lehins lehins force-pushed the jj/pparams-lens branch 7 times, most recently from 1774cd3 to 1253991 Compare December 28, 2022 20:06
@lehins lehins force-pushed the jj/pparams-lens branch 6 times, most recently from 4af026e to 4a148bf Compare January 10, 2023 13:38
@lehins lehins changed the title WIP: Add lenses to PParams, create a [Era]EraPParams class for each era that needs it Add lenses to PParams, create a [Era]EraPParams class for each era that needs it Jan 10, 2023
@lehins lehins marked this pull request as ready for review January 10, 2023 14:54
@lehins
Copy link
Collaborator

lehins commented Jan 10, 2023

Closing in favor of the work that was built atop of this PR: #3242

@lehins lehins closed this Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aniketd aniketd aniketd left review comments

@JaredCorduan JaredCorduan JaredCorduan left review comments

@lehins lehins lehins left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Switch PParams to use lenses instead of HasField
4 participants
@Soupstraw @lehins @aniketd @JaredCorduan