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

separate trie from util and make its dependencies into libs #6478

Merged
merged 2 commits into from
Sep 15, 2017
Merged

separate trie from util and make its dependencies into libs #6478

merged 2 commits into from
Sep 15, 2017

Conversation

Hawstein
Copy link
Contributor

@Hawstein Hawstein commented Sep 6, 2017

Part of #6418

Separate trie from util and make its dependencies into libs:

  • bytes
  • hashdb
  • memorydb
  • nibbleslice
  • nibblevec

@Hawstein Hawstein changed the title separate trie from util and make its dependencies into libs: separate trie from util and make its dependencies into libs Sep 6, 2017
@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 6, 2017
@Hawstein Hawstein force-pushed the trie-separated-from-util branch 4 times, most recently from 0545fc8 to 007ba1b Compare September 7, 2017 07:11
@Hawstein
Copy link
Contributor Author

Hawstein commented Sep 7, 2017 •
edited
Loading

Finally, all the errors are resolved! And it is ready for review.

I will work on the left tasks of #6418 after this PR is merged since they depend on the changes made by this PR.

@rphmeier
Copy link
Contributor

rphmeier commented Sep 8, 2017

could you rename the trie package to patricia_trie (open to suggestions). We can publish it on crates.io once that's done and it's merged.

rphmeier
rphmeier reviewed Sep 8, 2017
View reviewed changes
util/trie/Cargo.toml Outdated
rlp = { path = "../rlp" }
nibbleslice = { path = "../nibbleslice" }
nibblevec = { path = "../nibblevec" }
triehash = { path = "../triehash" }
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe triehash should go in the same crate, within an "ephemeral" module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I guess we can make it a separated PR if all of we agree to move it to this crate.

@Hawstein
Copy link
Contributor Author

Hawstein commented Sep 9, 2017 •
edited
Loading

could you rename the trie package to patricia_trie (open to suggestions). We can publish it on crates.io once that's done and it's merged.

@rphmeier Sure, patricia-trie seems like a reasonable name.

WDYT? @gavofyork @debris @tomusdrw @NikVolf

@rphmeier
Copy link
Contributor

@Hawstein This could use a merge

@rphmeier rphmeier added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 12, 2017
@Hawstein Hawstein force-pushed the trie-separated-from-util branch 2 times, most recently from ed42732 to 3fe5b36 Compare September 12, 2017 18:05
@Hawstein
Copy link
Contributor Author

Hawstein commented Sep 12, 2017 •
edited
Loading

@rphmeier Rebase master and rename trie.

Could you review it if you have time? Since it touches lots of files, it will easily have conflicts with other new PRs.

@5chdn 5chdn added A0-pleasereview 🤓 Pull request needs code review. P5-sometimesoon 🌲 Issue is worth doing soon. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Sep 12, 2017
debris
debris approved these changes Sep 14, 2017
View reviewed changes
Copy link
Collaborator

@debris debris left a comment

Choose a reason for hiding this comment

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

I'm just not 100% sure if nibblevec and nibbleslice need to be standalone libraries. Apart from that, lgtm.

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 14, 2017
@Hawstein
Copy link
Contributor Author

Hawstein commented Sep 15, 2017 •
edited
Loading

@gavofyork @debris @5chdn

Just rebased master. This can be merged without conflicts now.

@debris debris merged commit d69dd17 into openethereum:master Sep 15, 2017
@Hawstein Hawstein deleted the trie-separated-from-util branch September 15, 2017 07:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rphmeier rphmeier rphmeier left review comments

@debris debris debris approved these changes

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Hawstein @rphmeier @debris @gavofyork @5chdn