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

Tracking issue util refactor #6418

Closed
13 of 15 tasks
debris opened this issue Aug 30, 2017 · 20 comments · Fixed by #6690
Closed
13 of 15 tasks

Tracking issue util refactor #6418

debris opened this issue Aug 30, 2017 · 20 comments · Fixed by #6690
Labels
F6-refactor 📚 Code needs refactoring. F9-meta 🔮 A specific issue for grouping tasks or bugs of a specific category. M4-core ⛓ Core client code / Rust. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Milestone

Comments

@debris
Copy link
Collaborator

debris commented Aug 30, 2017

  • @Hawstein journaldb and rocksdb dependencies should be separated from util.
  • @Hawstein all trie traits from util should be separated from util.
  • migrations should be a library
  • @debris -> replace sha3::Hashable trait and all of usages of it with a simple function called hash
  • @debris -> this hash function should be a standalone library importing C impl of sha3 and tiny-keccak (for updatable sha3s)
  • @folsen util should not reexport bigint
  • @Hawstein util should not reexport ansi_term
  • @axelchalon -> util should not reexport HeapSizeOf
  • @Hawstein util should not reexport ParkingLot
  • @debris util should not reexport vector::*
  • @debris triehash should be separated from util
  • @Hawstein semantic_version should be separated from util
  • @Hawstein timer::PerfTimer should be a part of ethcore
  • @debris error::UtilError should be replaced by error_chain
  • remove public reexports of traits modules
  • flatten util subcrates (controversial)
@debris debris added F6-refactor 📚 Code needs refactoring. M4-core ⛓ Core client code / Rust. labels Aug 30, 2017
@rphmeier
Copy link
Contributor

I'm not convinced about flattening util subcrates. Why not group non-blockchain-specific utilities within a single folder?

@debris
Copy link
Collaborator Author

debris commented Aug 30, 2017

ok, let's keep all subcrates in util directory for now. We'll discuss it again someday

@debris
Copy link
Collaborator Author

debris commented Aug 30, 2017

I'll take

- [ ] replace sha3::Hashable trait and all of usages of it with a simple function called `hash`
- [ ] this `hash` function should be a standalone library importing C impl of sha3 and tiny-keccak (for updatable sha3s)

@NikVolf
Copy link
Contributor

NikVolf commented Aug 30, 2017

Also fixed hash should be crates.io library dependant on bigint and little more

@axelchalon
Copy link
Contributor

I'll take util should not reexport HeapSizeOf for now

@debris
Copy link
Collaborator Author

debris commented Aug 31, 2017

I will take triehash should be separated from util

@folsen
Copy link
Contributor

folsen commented Sep 1, 2017

I'll take util should not reexport bigint

@Hawstein
Copy link
Contributor

Hawstein commented Sep 1, 2017

I'll take util should not reexport ansi_term if you guys don't mind:)

@Hawstein
Copy link
Contributor

Hawstein commented Sep 1, 2017

I'll take one more: util should not reexport ParkingLot.

@debris
Copy link
Collaborator Author

debris commented Sep 1, 2017

@Hawstein thanks, merged!

@Hawstein
Copy link
Contributor

Hawstein commented Sep 1, 2017

I will take timer::PerfTimer should be a part of ethcore.

@Hawstein
Copy link
Contributor

Hawstein commented Sep 1, 2017

I will take semantic_version should be separated from util.

@Hawstein
Copy link
Contributor

Hawstein commented Sep 2, 2017

I will take

  • journaldb and rocksdb dependencies should be separated from util.
  • all trie traits from util should be separated from util.

@Hawstein
Copy link
Contributor

Hawstein commented Sep 3, 2017

@debris Should I move the journaldb, migration and trie out of util/src dir and put them in the util/ dir just like other libs. Seems that this is a reasonable way to arrange them.

@debris
Copy link
Collaborator Author

debris commented Sep 3, 2017

@debris Should I move the journaldb, migration and trie out of util/src dir and put them in the util/ dir just like other libs. Seems that this is a reasonable way to arrange them.

@Hawstein yes, this would be a great first step. Eventually, we would also like to decouple journaldb and rocksdb :)

@rphmeier
Copy link
Contributor

rphmeier commented Sep 3, 2017

@debris they are already decoupled to some degree via the KeyValueDB trait.

@5chdn 5chdn added the F9-meta 🔮 A specific issue for grouping tasks or bugs of a specific category. label Sep 4, 2017
@5chdn 5chdn added the P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible. label Sep 4, 2017
@debris
Copy link
Collaborator Author

debris commented Sep 5, 2017

I will take error::UtilError should be replaced by error_chain

@Hawstein
Copy link
Contributor

Hawstein commented Sep 5, 2017

@debris Hi, I am working on the all trie traits from util should be separated from util. Since it depends lots of stuffs in util/src, so I decide to move all the *.rs file in util/src out of the src dir and make it a lib. It needs to be done for other tasks too. Does it make sense for you?

@debris
Copy link
Collaborator Author

debris commented Sep 5, 2017

@Hawstein yes, I believe it's one of the last things needed ;)

@Hawstein
Copy link
Contributor

Hawstein commented Sep 6, 2017

@debris Great! Then I will take the following tasks too:

  • migrations should be a library
  • remove public reexports of traits modules

Since once I make those *.rs files into a lib, the above tasks are almost done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F6-refactor 📚 Code needs refactoring. F9-meta 🔮 A specific issue for grouping tasks or bugs of a specific category. M4-core ⛓ Core client code / Rust. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants