Skip to content

Catch up with amino 0.13.0#2690

Merged
ebuchman merged 5 commits intodevelopfrom
amino_explicit_len_prefix
Oct 25, 2018
Merged

Catch up with amino 0.13.0#2690
ebuchman merged 5 commits intodevelopfrom
amino_explicit_len_prefix

Conversation

@liamsi
Copy link
Contributor

@liamsi liamsi commented Oct 22, 2018

Depends on #2687
related: tendermint/go-amino#222
related: #2687
related: tendermint/go-amino#233

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

@liamsi liamsi changed the base branch from master to develop October 22, 2018 17:49
@liamsi liamsi force-pushed the amino_explicit_len_prefix branch from 8a71360 to 2fc5d86 Compare October 22, 2018 18:04
@liamsi liamsi mentioned this pull request Oct 22, 2018
4 tasks
buf = append(buf, part.Bytes...)
}
err := cdc.UnmarshalBinary(buf, block)
err := cdc.UnmarshalBinaryLengthPrefixedBinary(buf, block)
Copy link
Contributor Author

@liamsi liamsi Oct 22, 2018

Choose a reason for hiding this comment

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

Oh no, this should actually be named: UnmarshalBinaryLengthPrefixed instead 🙈 Not sure how me and two reviewers didn't see that. I'll delete the release 0.13.0 for now and add a commit to master that fixes that (and tag this again).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ebuchman
Copy link
Contributor

Should we merge #2687 first?

@liamsi
Copy link
Contributor Author

liamsi commented Oct 23, 2018

Yes, we should. Otherwise we'd probably have to redo the changes from #2687 here, too.

@liamsi liamsi changed the title [WIP]: Catch up with amino 0.13.0 Catch up with amino 0.13.0 Oct 23, 2018
@liamsi
Copy link
Contributor Author

liamsi commented Oct 23, 2018

Should be ready for review if circleci passes.

Copy link
Contributor

@ebuchman ebuchman left a comment

Choose a reason for hiding this comment

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

Cool, thanks.

Very clear now that we length prefix everything ;). We should open a separate issue to consider if maybe we shouldn't actually do that everywhere.

@ebuchman ebuchman merged commit 6643c5d into develop Oct 25, 2018
@ebuchman ebuchman deleted the amino_explicit_len_prefix branch October 25, 2018 01:34
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.

2 participants