-
Notifications
You must be signed in to change notification settings - Fork 156
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
Rename UTxOState fields without underscores #3108
Conversation
6ac47e9
to
c6bdf8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks wonderful to me, thank you @aniketd !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change and requires a bump in version. So, currently released version of cardano-ledger-shelley
is 0.1.0.0
, so we need to bump it up to 0.2.0.0
. Otherwise it looks great. This is a fairly new process, which we should all adopt
Also, we need to add a changelog to all of our packages and start placing changes there, instead of top level CHNAGELOG file. @aniketd This could be your next task, which you could do in a follow up PR
Should we also release the new version to cHaP? |
No, we don't need to make a release for every PR. Only when a downstream library is ready to integrate and use the new version, we should make a release
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. It just needs a conflict resolution and I suggest to squash all commits into one
Update CHANGELOG Run formatter Bump cardano-ledger-shelley version to 0.2.0.0
53b6cc9
to
c882c04
Compare
Done! |
There was a problem hiding this 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@aniketd - this is the most highly approved PR I've seen! 😄 🎉 |
Partially addresses #3061
- [ ] Add DEPRECATED warning for all renamed fieldsThey won't be useful as setters.- [ ] Open PRs against downstream packages to not use the deprecated accessors.The CHANGELOG update could be enough.