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

Use workspace.dependenceis to ensure consistent versions of common crates #2065

Closed
wants to merge 3 commits into from

Conversation

jsdw
Copy link
Contributor

@jsdw jsdw commented Oct 27, 2023

Description

This uses the [workspace.dependencies] field to give some consistency over versions of common crates used all over the place and make keeping their versions in sync easier.

For now, I just started with two common crates, scale-info and parity-scale-codec. If this is something that's wanted, I can add some more common crates to the list, or else others can add to the list.

Downsides

parity-scale-codec is renamed to codec in many places. Owing to a quirk of workwspace dependencies, it must be declared in the workspace as:

codec = { package = "parity-scale-codec", version = "3.6.5" }

Owing to how the macro discovers which crate path to use, it currently needs the package field to be duplicated as in:

codec = { package = "parity-scale-codec", workspace = true, ... }

This leads to a bunch of warnings. It could be fixed in parity-scale-codec with a bit of effort.

The other thing leading to warnings is that eg default-features = false should be added to the workspace dependencies, else it is ignored in the use site. Fixing any fallout from that would lead to a little churn perhaps.

Unknowns

Does this break the tooling to publish crates automatically, or any other CI workflows?

@jsdw jsdw added the T11-documentation This PR/Issue is related to documentation. label Oct 27, 2023
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable-int
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4111419

@jsdw
Copy link
Contributor Author

jsdw commented Oct 27, 2023

I thought I'd havea go at doing this, because it's generally worked well for us in Subxt (though the number of warnings is small and quite tolerable). I think to do it in polkadot-sdk would make consistyfing the versions nicer, but right now there are just too many warnings and things. I might look at fixing parity-scale-codec to remove the ones it leads to, but until then I don't think this is worthwhile

@jsdw jsdw closed this Oct 27, 2023
@bkchr bkchr deleted the jsdw-use-workspace-deps branch October 27, 2023 18:48
@jsdw
Copy link
Contributor Author

jsdw commented Oct 30, 2023

Note to anybody reading this; actually parity-scale-codec doesn't need the "package = 'parity-scale-codec'" everywhere any more once the next version is released, owing to paritytech/parity-scale-codec#535. Yay!

github-merge-queue bot pushed a commit that referenced this pull request Feb 12, 2024
Changes (partial #994):
- Set log to `0.4.20` everywhere
- Lift `log` to the workspace

Starting with a simpler one after seeing
#2065 from @jsdw.
This sets the `default-features` to `false` in the root and then
overwrites that in each create to its original value. This is necessary
since otherwise the `default` features are additive and its impossible
to disable them in the crate again once they are enabled in the
workspace.

I am using a tool to do this, so its mostly a test to see that it works
as expected.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
Changes (partial paritytech#994):
- Set log to `0.4.20` everywhere
- Lift `log` to the workspace

Starting with a simpler one after seeing
paritytech#2065 from @jsdw.
This sets the `default-features` to `false` in the root and then
overwrites that in each create to its original value. This is necessary
since otherwise the `default` features are additive and its impossible
to disable them in the crate again once they are enabled in the
workspace.

I am using a tool to do this, so its mostly a test to see that it works
as expected.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 26, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 27, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
bkchr pushed a commit that referenced this pull request Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T11-documentation This PR/Issue is related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants