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

Change default encoding to binary, instead of hex #3

Merged
merged 2 commits into from
Aug 2, 2017

Conversation

hayesgm
Copy link
Contributor

@hayesgm hayesgm commented Aug 1, 2017

This patch makes a few changes, but most importantly, we define a breaking change to make the default encoding of an RLP structure the binary encoding (e.g. <<0x55, 0x66, 0x77>>) instead of hex (e.g. "556677"). This is because the binary encoding is faster and more appropriate when encoding and decoding RLP from the blockchain. We allow a user of ExRLP to return to hex encoding by specifying encoding: :hex in the options hash.

Additionally, we merge the test cases from another RLP library and add typespecs for dialyzer.

This patch makes a few changes, but most importantly, we define a breaking change to make the default encoding of an RLP structure the binary encoding (e.g. <<0x55, 0x66, 0x77>>) instead of hex (e.g. "556677"). This is because the binary encoding is faster and more appropriate when encoding and decoding RLP from the blockchain. We allow a user of ExRLP to return to hex encoding by specifying `encoding: :hex` in the options hash.

Additionally, we merge the test cases from another RLP library and add typespecs for dialyzer.
@hayesgm hayesgm requested a review from ayrat555 August 1, 2017 07:38
Copy link
Member

@ayrat555 ayrat555 left a comment

Choose a reason for hiding this comment

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

@hayesgm good work! It's great that you added typespecs to all methods.

@ayrat555
Copy link
Member

ayrat555 commented Aug 1, 2017

@hayesgm can you bump version and describe your changes in CHANGELOG, please?

We've made a non-backwards compatable change (changing the default encoding to binary instead of hex), and thus we'll bump the version up a minor (since we're still less than 1.0.0). Added notes on what has changed.
@hayesgm hayesgm merged commit 262546b into master Aug 2, 2017
masonforest pushed a commit that referenced this pull request Apr 13, 2018
Change default encoding to binary, instead of hex
@ayrat555 ayrat555 deleted the hayesgm/binary-encoding branch April 16, 2018 10:54
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