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

Refactor: Don't reexport bigint from util #6459

Merged
merged 6 commits into from
Sep 5, 2017
Merged

Conversation

folsen
Copy link
Contributor

@folsen folsen commented Sep 4, 2017

Part of #6418

This PR removes

pub use bigint::prelude::*;		
pub use bigint::hash;

from util/src/lib.rs and then changes all the affected places to directly import what's needed from bigint.

@folsen folsen requested a review from debris September 4, 2017 14:39
@parity-cla-bot
Copy link

It looks like @folsen signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@NikVolf
Copy link
Contributor

NikVolf commented Sep 4, 2017

So now you are reexporting things from ethcore_bigint?

I think better way is to use U256 directly from bigint crate on crates.io where possible

@debris
Copy link
Collaborator

debris commented Sep 4, 2017

I think better way is to use U256 directly from bigint crate on crates.io where possible

I believe this could be a separate pr. For now we don't need this that much, cause in most places we use both anyway :)

@NikVolf
Copy link
Contributor

NikVolf commented Sep 4, 2017

@debris
Ok, but almost the same job will be performed twice then ;)

@debris
Copy link
Collaborator

debris commented Sep 4, 2017

@NikVolf nah, second time will be easier, cause there won't be any wildcard imports

something like that

s/bigint::hash::H256/ethcore_bigint::H256
s/bigint::prelude::U256/bigint::U256 

plus filling out missing imports should be enough

@folsen
Copy link
Contributor Author

folsen commented Sep 4, 2017

second time will be easier, cause there won't be any wildcard imports

Yeah actually one of the most annoying things here were glob imports, should we put in a rule to never use wildcard imports? Would make sense to me.

@debris
Copy link
Collaborator

debris commented Sep 4, 2017

@folsen Definitely! Another one for #6443

@rphmeier
Copy link
Contributor

rphmeier commented Sep 4, 2017

We've cut down a ton on the number of glob imports, but use util::* is still pretty common. I don't think any new glob imports are being introduced but a bunch are still hanging around.

folsen and others added 5 commits September 4, 2017 18:32
# Conflicts:
#	dapps/src/tests/helpers/registrar.rs
#	ethcore/evm/src/interpreter/shared_cache.rs
#	ethcore/light/src/client/header_chain.rs
#	ethcore/light/src/client/mod.rs
#	ethcore/light/src/net/mod.rs
#	ethcore/light/src/on_demand/request.rs
#	ethcore/light/src/on_demand/tests.rs
#	ethcore/light/src/provider.rs
#	ethcore/node_filter/src/lib.rs
#	ethcore/src/block.rs
#	ethcore/src/blockchain/blockchain.rs
#	ethcore/src/client/test_client.rs
#	ethcore/src/engines/authority_round/mod.rs
#	ethcore/src/engines/basic_authority.rs
#	ethcore/src/engines/mod.rs
#	ethcore/src/engines/tendermint/mod.rs
#	ethcore/src/engines/validator_set/contract.rs
#	ethcore/src/engines/validator_set/multi.rs
#	ethcore/src/engines/validator_set/safe_contract.rs
#	ethcore/src/engines/vote_collector.rs
#	ethcore/src/miner/external.rs
#	ethcore/src/miner/miner.rs
#	ethcore/src/miner/service_transaction_checker.rs
#	ethcore/src/miner/work_notify.rs
#	ethcore/src/pod_account.rs
#	ethcore/src/pod_state.rs
#	ethcore/src/snapshot/block.rs
#	ethcore/src/snapshot/consensus/work.rs
#	ethcore/src/snapshot/mod.rs
#	ethcore/src/snapshot/service.rs
#	ethcore/src/spec/spec.rs
#	ethcore/src/state/backend.rs
#	ethcore/src/trace/db.rs
#	ethcore/src/verification/queue/mod.rs
#	ethcore/src/verification/verification.rs
#	parity/informant.rs
#	rpc/src/v1/helpers/dispatch.rs
#	rpc/src/v1/helpers/light_fetch.rs
#	rpc/src/v1/helpers/signing_queue.rs
#	rpc/src/v1/impls/eth.rs
#	rpc/src/v1/impls/eth_filter.rs
#	rpc/src/v1/impls/eth_pubsub.rs
#	rpc/src/v1/impls/light/eth.rs
#	rpc/src/v1/impls/signing.rs
#	rpc/src/v1/tests/helpers/miner_service.rs
#	rpc/src/v1/tests/helpers/snapshot_service.rs
#	rpc/src/v1/tests/helpers/sync_provider.rs
#	rpc/src/v1/tests/mocked/eth.rs
#	stratum/src/lib.rs
#	sync/src/blocks.rs
#	sync/src/chain.rs
#	sync/src/light_sync/mod.rs
#	sync/src/tests/helpers.rs
#	sync/src/tests/snapshot.rs
#	updater/src/updater.rs
#	util/src/lib.rs
#	util/triehash/src/lib.rs
@debris
Copy link
Collaborator

debris commented Sep 5, 2017

I'll merge before more conflicts are introduced

@debris debris added the A8-looksgood 🦄 Pull request is reviewed well. label Sep 5, 2017
@debris debris merged commit c49becc into master Sep 5, 2017
@debris debris deleted the fo-6418-dont-export-bigint branch September 5, 2017 10:38
@debris debris mentioned this pull request Sep 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants