Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lift all dependencies (the big one) #4716

Merged
merged 19 commits into from
Jun 24, 2024
Merged

Lift all dependencies (the big one) #4716

merged 19 commits into from
Jun 24, 2024

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Jun 5, 2024

After preparing in #4633, we can lift also all internal dependencies up to the workspace.

This does not actually change anything, but uses workspace = true for all dependencies. You can check it with:

git checkout -q $(git merge-base oty-lift-all-deps origin/master)
cargo tree -e features > master.out

git checkout -q oty-lift-all-deps
cargo tree -e features > new.out
diff master.out new.out

It did not yet lift 100% of dependencies, some inside of target.* or some that had conflicting aliases introduced recently. But i will do these together in a follow-up with CI checks.

Can be reproduced with zepter: zepter transpose d lift-to-workspace "regex:.*" --version-resolver highest --skip-package "polkadot-sdk" --ignore-errors --fix.

ggwpez added 3 commits June 5, 2024 19:20
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez requested review from athei, acatangiu, sorpaas, andresilva, cheme, a team and koute as code owners June 5, 2024 17:41
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 5, 2024 17:42
@ggwpez ggwpez added the R0-silent Changes should not be mentioned in any release notes label Jun 5, 2024
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Found some incorrect things.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez requested review from a team as code owners June 10, 2024 12:48
ggwpez added 12 commits June 10, 2024 15:11
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
rand_core = { version = "0.6.2" }
rand_distr = { version = "0.4.3" }
rand_pcg = { version = "0.3.1" }
rayon = { version = "1.5.1" }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rayon = { version = "1.5.1" }
rayon = "1.5.1"

Would have been nice for all of them, but just a nitty nitpick.

Generally some CI check to ensure we always add stuff with workspace = true would be good, but can come later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would have been nice for all of them, but just a nitty nitpick.

👍

Generally some CI check to ensure we always add stuff with workspace = true would be good, but can come later.

There are some checks in place that should prevent the most obvious cases, but i did not yet check yet if its actually bullet-proof.

ggwpez added 2 commits June 24, 2024 12:41
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@bkchr bkchr added this pull request to the merge queue Jun 24, 2024
Merged via the queue into master with commit 8efa054 Jun 24, 2024
156 of 158 checks passed
@bkchr bkchr deleted the oty-lift-all-deps branch June 24, 2024 11:56
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit added 3 .wasm files, two of which are in the root of the repository, yet commit message simply says "Taplo".

Why?

Copy link
Member

Choose a reason for hiding this comment

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

Haha nice find :D #5052

github-merge-queue bot pushed a commit that referenced this pull request Jul 18, 2024
There are slight changes required, after
#4716.

In order to follow the convention in the templates, we need to specify
the internal template dependencies (runtime and a pallet) in the main
`Cargo.toml`.

Additionally, there are now dependencies with `path` in the main
`Cargo.toml`, so we add a step with `psvm` to change those references to
crate versions.
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
After preparing in paritytech#4633,
we can lift also all internal dependencies up to the workspace.

This does not actually change anything, but uses `workspace = true` for
all dependencies. You can check it with:
```bash
git checkout -q $(git merge-base oty-lift-all-deps origin/master)
cargo tree -e features > master.out

git checkout -q oty-lift-all-deps
cargo tree -e features > new.out
diff master.out new.out
```

It did not yet lift 100% of dependencies, some inside of `target.*` or
some that had conflicting aliases introduced recently. But i will do
these together in a follow-up with CI checks.

Can be reproduced with [zepter](https://github.com/ggwpez/zepter/):
`zepter transpose d lift-to-workspace "regex:.*" --version-resolver
highest --skip-package "polkadot-sdk" --ignore-errors --fix`.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
There are slight changes required, after
paritytech#4716.

In order to follow the convention in the templates, we need to specify
the internal template dependencies (runtime and a pallet) in the main
`Cargo.toml`.

Additionally, there are now dependencies with `path` in the main
`Cargo.toml`, so we add a step with `psvm` to change those references to
crate versions.
sfffaaa pushed a commit to peaqnetwork/polkadot-sdk that referenced this pull request Dec 27, 2024
After preparing in paritytech#4633,
we can lift also all internal dependencies up to the workspace.

This does not actually change anything, but uses `workspace = true` for
all dependencies. You can check it with:
```bash
git checkout -q $(git merge-base oty-lift-all-deps origin/master)
cargo tree -e features > master.out

git checkout -q oty-lift-all-deps
cargo tree -e features > new.out
diff master.out new.out
```

It did not yet lift 100% of dependencies, some inside of `target.*` or
some that had conflicting aliases introduced recently. But i will do
these together in a follow-up with CI checks.

Can be reproduced with [zepter](https://github.com/ggwpez/zepter/):
`zepter transpose d lift-to-workspace "regex:.*" --version-resolver
highest --skip-package "polkadot-sdk" --ignore-errors --fix`.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants