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

Deduplicating crate dependencies (part 3 of n) #11614

Merged
merged 3 commits into from
Apr 9, 2020

Conversation

shergin
Copy link

@shergin shergin commented Apr 8, 2020

The change includes:

tiny-keccak
tokio-timer
textwrap
winapi
rand
rand_xorshift
scopeguard
url

#11468

…timer`, and co) (openethereum#11613)

The change includes:
```
tiny-keccak
tokio-timer
textwrap
winapi
rand
rand_xorshift
scopeguard
url
```
@shergin shergin changed the title Deduplicating crate dependencies (part 3 of n) (#11613) Deduplicating crate dependencies (part 3 of n) Apr 8, 2020
Copy link
Collaborator

@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.

LGTM. I'm surprised it was so easy to upgrade tiny_keccak; for some reason I thought it would have been much more busy work. I guess most uses are in the parity-common crates.

@@ -31,8 +31,11 @@ impl Hasher for KeccakHasher {
type StdHasher = PlainHasher;
const LENGTH: usize = 32;
fn hash(x: &[u8]) -> Self::Out {
use tiny_keccak::Hasher;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use tiny_keccak::{Keccak, Hasher}; on line 23?

Copy link
Author

Choose a reason for hiding this comment

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

I am n00b in Rust. I tried that originally and got compiler error saying something like "using trait implementations ... blah blah... must be in the scope", so I moved that to the scope and the error is gone. I will investigate why exactly (but it seems correct, ¯\_(ツ)_/¯).

@ordian
Copy link
Collaborator

ordian commented Apr 8, 2020

LGTM. I'm surprised it was so easy to upgrade tiny_keccak; for some reason I thought it would have been much more busy work. I guess most uses are in the parity-common crates.

tiny-keccak 1.5.0 is still there (used in ethbloom, keccak-hash, keccak-hasher, parity-secretstore)

Copy link
Collaborator

@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.

Good job deduplicating tokio-timer, textwrap and slab 💪

@ordian ordian added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. M5-dependencies 🖇 Dependencies. labels Apr 8, 2020
shergin and others added 2 commits April 8, 2020 10:04
Co-Authored-By: Andronik Ordian <write@reusable.software>
Co-Authored-By: Andronik Ordian <write@reusable.software>
@shergin
Copy link
Author

shergin commented Apr 8, 2020

From my understanding, most of remaining duplicating dependencies are in 3rd-party deps (or maybe I am missing something), not sure what we should/can do with that.

@ordian
Copy link
Collaborator

ordian commented Apr 8, 2020

From my understanding, most of remaining duplicating dependencies are in 3rd-party deps (or maybe I am missing something), not sure what we should/can do with that.

We still can deduplicate crates like syn and proc-macro2 by upgrading dependencies (ethabi, logos and validator -derive) and tiny-keccak (see the list above).

@shergin
Copy link
Author

shergin commented Apr 9, 2020

Thank you folks!
I will try to upgrade deps in a separate PR.

@ordian ordian merged commit a5820b6 into openethereum:master Apr 9, 2020
dvdplm added a commit that referenced this pull request Apr 14, 2020
* master: (25 commits)
  Update .gitmodules (#11628)
  ethcore/res: activate ecip-1088 phoenix on classic (#11598)
  Upgrade parity-common deps to latest (#11620)
  Fix Goerli syncing (#11604)
  deps: switch to upstream ctrlc (#11617)
  Deduplicating crate dependencies (part 3 of n) (#11614)
  Deduplicating crate dependencies (part 2 of n, `slab`) (#11613)
  Actually save ENR on creation and modification (#11602)
  Activate POSDAO on xDai chain and update bootnodes (#11610)
  Activate on-chain randomness in POA Core (#11609)
  Deduplicating crate dependencies (part 1 of n) (#11606)
  Update enodes for POA Sokol (#11611)
  Remove .git folder from dogerignore file so vergen library can get build date and commit hash in the binary generatio vergen library can get build date and commit hash in the binary generation (#11608)
  Reduced gas cost for static calls made to precompiles EIP2046/1352 (#11583)
  [easy] `ethcore-bloom-journal` was renamed to `accounts-bloom` (#11605)
  Use serde_json to export hardcoded sync (#11601)
  Node Discovery v4 ENR Extension (EIP-868) (#11540)
  Fix compile warnings (#11595)
  Update version to 3.0.0-alpha.1 (#11592)
  ethcore/res: bump canon fork hash for mordor and kotti testnets (#11584)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. M5-dependencies 🖇 Dependencies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants