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

Refactor delegationTransition #3129

Merged
merged 4 commits into from
Nov 25, 2022
Merged

Refactor delegationTransition #3129

merged 4 commits into from
Nov 25, 2022

Conversation

WhatisRT
Copy link
Contributor

@WhatisRT WhatisRT commented Nov 8, 2022

There was a lot of code copy which was in the way of @teodanciu and me figuring out what changes if allowMIRTransfer is true or not. We took this as an opportunity for some refactoring.

@WhatisRT WhatisRT requested a review from teodanciu November 8, 2022 14:41
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.

Love the deduplication. ❤️ Suggested even further improvement

@teodanciu
Copy link
Contributor

I'll make this work and adapt the suggestions when I'm back on Friday!

@teodanciu teodanciu force-pushed the andre/deleg-refactor branch 4 times, most recently from 64919db to 5b2cd23 Compare November 16, 2022 22:06
Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

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

great cleanup, thank you @teodanciu !

The darwin cicero build is failing with something I don't understand:

F copying 2 paths...
F cardano-protocol-tpraos-lib-cardano-protocol-tpraos> Connection to 10.10.0.2 closed by remote host
F error: unexpected end-of-file

@teodanciu teodanciu force-pushed the andre/deleg-refactor branch from bf5c6e9 to bd12967 Compare November 22, 2022 08:50
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.

Thank you for reducing duplication. Great stuff!!

Just one minor stylistic comment

WhatisRT and others added 3 commits November 24, 2022 15:49
There was a lot of code copy which was in the way of @teodanciu and me
figuring out what changes if `allowMIRTransfer` is true or not. We
took this as an opportunity for some refactoring.

Co-authored-by: teodanciu <teodora.danciu@tweag.io>
to avoid duplication and apply suggested further refactorings
Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
@teodanciu teodanciu force-pushed the andre/deleg-refactor branch from 846fca8 to 9b8e195 Compare November 24, 2022 13:49
@teodanciu teodanciu merged commit 8d5d787 into master Nov 25, 2022
@iohk-bors iohk-bors bot deleted the andre/deleg-refactor branch November 25, 2022 10:22
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.

4 participants