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

Get bigint on crates.io #2078

Merged
merged 2 commits into from
Sep 15, 2016
Merged

Get bigint on crates.io #2078

merged 2 commits into from
Sep 15, 2016

Conversation

rphmeier
Copy link
Contributor

As ethcore-bigint

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 13, 2016
@tomusdrw
Copy link
Collaborator

What's the benefit if the code is in a single repo anyway?

@rphmeier
Copy link
Contributor Author

rphmeier commented Sep 13, 2016

It makes it easier to observe changes -- personally I hate to review a PR which is just a version bump + Cargo.lock hash change, because it almost always means that this "insubstantial" PR is just a mask for one which hasn't been properly reviewed.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 84.671% when pulling 14c0c24 on bigint_crates into 341e064 on master.

@tomusdrw
Copy link
Collaborator

Oh, yeah - I agree. My point is why do we change references to ethcore-bigint from ethcore-bigint = {path = "../util/bigint"} to ethcore-bigint = "<crates-version>"

Right now someone will make a change in util/bigint and it will not be used when compiling parity so we won't be even able to figure out that something is broken.

@arkpar
Copy link
Collaborator

arkpar commented Sep 13, 2016

Agree with @tomusdrw. Local references should use relative path.

@rphmeier
Copy link
Contributor Author

my reasoning behind using crates.io version was to ensure that changes actually get propagated to crates.io. if not, the crate might stagnate more than necessary.

but i suppose the relative path is probably better

@arkpar arkpar added the A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. label Sep 14, 2016
@rphmeier rphmeier removed the A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. label Sep 14, 2016
@rphmeier
Copy link
Contributor Author

Another reason, which only just occurred to me now, is that you can't have local dependencies for published crates.

As we may want to publish an rlp and trie crate, they will need to reference concrete crates.io versions. I think cargo workspaces will smooth this over a bit.

@rphmeier
Copy link
Contributor Author

Travis failure seems spurious, it's very strange. It can't fetch the source for https://github.com/ethcore/hyper for some reason.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 85.268% when pulling 9dbc49b on bigint_crates into 2ba4968 on master.

@rphmeier rphmeier merged commit c16bf7f into master Sep 15, 2016
@debris debris deleted the bigint_crates branch September 15, 2016 15:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants