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

Bump cardano-ledger-specs and fix breaking change #3282

Merged
merged 1 commit into from
Jul 30, 2021

Conversation

kk-hainq
Copy link
Contributor

No description provided.

nc6
nc6 previously requested changes Jul 30, 2021
Copy link
Contributor

@nc6 nc6 left a comment

Choose a reason for hiding this comment

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

You'll need to fix the sha hash for the ledger repo. I've suggested the fix.

cabal.project Outdated Show resolved Hide resolved
@kk-hainq
Copy link
Contributor Author

kk-hainq commented Jul 30, 2021

You'll need to fix the sha hash for the ledger repo. I've suggested the fix.

Sorry I was careless and used cabal instead of nix for speed. Have fixed as suggested!

@nfrisby
Copy link
Contributor

nfrisby commented Jul 30, 2021

Hi @kk-hainq , thanks for fixing this for us.

Please squash your two commits into one before merging this PR. Otherwise, LGTM. Thanks.

@kk-hainq
Copy link
Contributor Author

Hi @kk-hainq , thanks for fixing this for us.

Please squash your two commits into one before merging this PR. Otherwise, LGTM. Thanks.

I have, thanks!

Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

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

Thanks much. Approving. And I'll merge too.

@nfrisby nfrisby dismissed nc6’s stale review July 30, 2021 17:29

The concern was addressed.

@nfrisby
Copy link
Contributor

nfrisby commented Jul 30, 2021

bors r+

@nfrisby
Copy link
Contributor

nfrisby commented Jul 30, 2021

Hi @kk-hainq. Thanks again for the helpful commit.

I was a bit lax this time, since this commit is so simple. But generally -- and usually even for a small one -- we'd like to have a few more details in the commit message and a non-empty PR description. (The description can even just say "Please see the commit messages" eg).

Thanks again!

@kk-hainq
Copy link
Contributor Author

Hi @kk-hainq. Thanks again for the helpful commit.

I was a bit lax this time, since this commit is so simple. But generally -- and usually even for a small one -- we'd like to have a few more details in the commit message and a non-empty PR description. (The description can even just say "Please see the commit messages" eg).

Thanks again!

Yes sir. I should have at least added a reference to the breaking commit in ledger. Will add more information next time!

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 30, 2021

@iohk-bors iohk-bors bot merged commit e9cda57 into IntersectMBO:master Jul 30, 2021
@kk-hainq kk-hainq deleted the update_ledger_specs branch July 30, 2021 17:40
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.

None yet

3 participants