-
Notifications
You must be signed in to change notification settings - Fork 748
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
Conversation
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
There was a problem hiding this 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.
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" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
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.
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
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.
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>
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.
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>
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: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
.