Skip to content

[rlp] Add no_std support #206

Merged
ordian merged 11 commits into
paritytech:masterfrom
koushiro:rlp-no-std
Sep 5, 2019
Merged

[rlp] Add no_std support #206
ordian merged 11 commits into
paritytech:masterfrom
koushiro:rlp-no-std

Conversation

@koushiro
Copy link
Copy Markdown
Contributor

@koushiro koushiro commented Aug 29, 2019 •
edited by ordian
Loading

Signed-off-by: koushiro koushiro.cqx@gmail.com

What have I changed?

  • Add no_std support for rlp crate
  • Fix some warnings
  • Convert rlp benches to criterion

related issue: #167, closes #171

@koushiro
Add no_std support for rlp
8de5d15 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Remove hex-literal
b11a623 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Aug 29, 2019

Sorry to reply so late. I will split #167 into multiple PRs. PTAL @dvdplm

@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Sep 3, 2019

Does anyone can review my code?

ordian
ordian approved these changes Sep 3, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@ordian ordian left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Comment thread .travis.yml Outdated
@ordian ordian requested a review from dvdplm September 4, 2019 11:04
@koushiro
Merge remote-tracking branch 'upstream/master' into rlp-no-std
507671a 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Fix some warnings
78e98da 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Remove std feature from primitive-types rlp feature
ef9d729 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
dvdplm
dvdplm reviewed Sep 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Overall it looks good.

We have this ongoing (implicit) discussion about rustc-hex. It is a low-priority long-term goal to remove it from all Parity projects so the changes here that re-introduces it are a problem. Can you elaborate on why you seem to dislike hex-literal and prefer rustc-hex?

@koushiro
Improve benchmark
2bd7d7c 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Sep 5, 2019

Overall it looks good.

We have this ongoing (implicit) discussion about rustc-hex. It is a low-priority long-term goal to remove it from all Parity projects so the changes here that re-introduces it are a problem. Can you elaborate on why you seem to dislike hex-literal and prefer rustc-hex?

I just want to reduce the number of crates, if rustc-hex will be removed in the future, then I will revert it.

@koushiro
Revert hex-literal
2a86b5e 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Fix impl-rlp
65ef685 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Sep 5, 2019

@dvdplm I want to know if you will release a new version for rlp crate on crates.io

@dvdplm
Copy link
Copy Markdown
Contributor

dvdplm commented Sep 5, 2019

@koushiro we will. Our policy is to test changes in parity-ethereum before releasing new versions. Once we have a PR there that passes all tests I'm fine making a release. Works for you?

@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Sep 5, 2019

@dvdplm Thanks for your response, it works for me :)

@niklasad1
Copy link
Copy Markdown
Contributor

niklasad1 commented Sep 5, 2019

@koushiro can you please fix the conflicts? Then we can merge it nice work 👍

@koushiro
Merge remote-tracking branch 'upstream/master' into rlp-no-std
286f137 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Sep 5, 2019

@niklasad1 Done :)

@ordian ordian merged commit 63b6ee3 into paritytech:master Sep 5, 2019
@koushiro koushiro deleted the rlp-no-std branch September 5, 2019 15:55
ordian pushed a commit that referenced this pull request Sep 6, 2019
Merge branch 'master' into ao-kvdb-indexeddb
1e89da3 
* master:
  [triehash] Migrate to 2018 edition (#214)
  [rlp] Add no_std support  (#206)
@niklasad1 niklasad1 mentioned this pull request Oct 26, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@dvdplm dvdplm dvdplm approved these changes

@ordian ordian ordian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A8-looksgood

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[rlp] convert benches to criterion

4 participants

@koushiro @dvdplm @niklasad1 @ordian