Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

rlp module refactor #8033

Closed
debris opened this issue Mar 1, 2018 · 6 comments
Closed

rlp module refactor #8033

debris opened this issue Mar 1, 2018 · 6 comments
Assignees
Labels
F6-refactor 📚 Code needs refactoring. P5-sometimesoon 🌲 Issue is worth doing soon. Q2-easy 💃 Can be fixed by copy and pasting from StackOverflow. Q9-epic 🌪 Can only be fixed by John Skeet.
Milestone

Comments

@debris
Copy link
Collaborator

debris commented Mar 1, 2018

should fix issues like #8031 or #8032

  • rlp::decode should return Result<_, DecoderError>
  • get rid of Rlp
  • UntrustedRlp should be renamed to Rlp
@debris debris added F6-refactor 📚 Code needs refactoring. Q9-epic 🌪 Can only be fixed by John Skeet. Q2-easy 💃 Can be fixed by copy and pasting from StackOverflow. labels Mar 1, 2018
@debris
Copy link
Collaborator Author

debris commented Mar 1, 2018

issues is marked as easy and epic, cause it's easy, but time consuming

@5chdn 5chdn added this to the 1.11 milestone Mar 1, 2018
@5chdn 5chdn added the P5-sometimesoon 🌲 Issue is worth doing soon. label Mar 1, 2018
@ascjones
Copy link
Contributor

ascjones commented Mar 6, 2018

I'm looking at this

@ascjones
Copy link
Contributor

ascjones commented Mar 20, 2018

This is being split across several PRs, since lots of changes

Phases:

  • Replace 'trusted' usages of Rlp with UntrustedRlp and handle Results, unsafely unwrapping where appropriate
  • Convert ethcore views to use UntrustedRlp and handle Results
  • Delete legacy Rlp and rename UntrustedRlp to Rlp
  • Change decode to return Result<_,DecoderError>

@ascjones
Copy link
Contributor

Just the last bit needs doing now, changing decode to return Result<_, DecoderError>. I'll leave that here for now, I've unassigned myself for now so anyone else can pick it up or I'll come back to it later.

@5chdn 5chdn modified the milestones: 1.11, 1.12 Apr 24, 2018
@dvdplm
Copy link
Collaborator

dvdplm commented May 1, 2018

I'll have a stab at the decode changes. :)

@ascjones
Copy link
Contributor

ascjones commented May 9, 2018

All done!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F6-refactor 📚 Code needs refactoring. P5-sometimesoon 🌲 Issue is worth doing soon. Q2-easy 💃 Can be fixed by copy and pasting from StackOverflow. Q9-epic 🌪 Can only be fixed by John Skeet.
Projects
None yet
Development

No branches or pull requests

4 participants