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

Setting versions locally towards 2.0 release #4404

Merged
merged 9 commits into from
Dec 17, 2019
Merged

Conversation

gnunicorn
Copy link
Contributor

@gnunicorn gnunicorn commented Dec 16, 2019

In order to allow to publish, path-dependencies must have a matching version specified, too. This PR attempts to make us ready for a 2.0 releasing to crates by :

  • cleaning up the path-definitions, move the version to the front and path to last place in all Cargo.toml
  • set the version on all path-defined dependencies
  • put sc-network and *consensus* versions into 0.8.0, incl pallet-aura and pallet-babe
  • bump other pallets to 2.0

refs #4099

@gnunicorn gnunicorn requested a review from gavofyork December 16, 2019 13:43
@gnunicorn gnunicorn added the A0-please_review Pull request needs code review. label Dec 16, 2019
@gnunicorn gnunicorn added this to the 2.0 milestone Dec 16, 2019
@gnunicorn gnunicorn added the A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). label Dec 16, 2019
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good, but it seems that some versions are still missing.

node-template-runtime = { path = "runtime" }
sp-runtime = { path = "../../primitives/runtime" }
sc-basic-authority = { path = "../../client/basic-authorship"}
sp-io = { version = "2.0.0", path = "../../primitives/io" }
Copy link
Member

Choose a reason for hiding this comment

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

why set versions as well as path? it's just redundant additional tedious data to maintain isn't it?

Copy link
Contributor Author

@gnunicorn gnunicorn Dec 17, 2019

Choose a reason for hiding this comment

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

Unfortunately, defining the version is a requirement to publish on crates.io. Cargo doesn't have any publish --all and won't resolve path to figure out which version to use–in general its publishing helper for workspaces are pretty rudimentary. On the other side path is obviously a lot easier and nicer for development.

Fortunately, because cargo uses semver this isn't much of a deal. We can just define version = "2.0.0" and even if we bump the minor or patch version of that crate to make a new release, this doesn't need any update on this crate. Cargo just will figure out the best version on cargo update (or cargo build if first time). Our internal Cargo.lock will also update without problems. This also allows us to do version updates more independently per crate, like updating network and consensus and apply the changes to the crates that follow without having to publish a breaking change on these, if those dependencies don't alter their externally perceived API.

Having both is the common way to stay sane, and is what we do in the libp2p workspace repo and common practice on other workspace repos, too.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@gnunicorn gnunicorn requested a review from tomusdrw December 17, 2019 10:22
@gavofyork gavofyork merged commit 8db5194 into master Dec 17, 2019
@gavofyork gavofyork deleted the ben-versioninig-2.0 branch December 17, 2019 14:05
sorpaas pushed a commit that referenced this pull request Nov 20, 2020
* clean up cargo.toml syntax

* bumping versions to 2.0

* bump networking to 0.8

* move consensus down to 0.8

* bump consensus pallets to 0.8.0, too

* Upping babe and aura pallets

* add remaining, missing version definitions

* missed some
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants