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

Align few crates versions #4374

Closed
wants to merge 94 commits into from
Closed

Conversation

jasl
Copy link
Contributor

@jasl jasl commented May 3, 2024

Closes #3414

@bkchr Here it is, I miss that DependentBot breaks compiling.

I also found there are a few dependencies in the codebase that use different versions.
This PR aims to align them to the highest version (in the code base)

I separate them into individual commits, so I can remove the changes you don't like

@paritytech-review-bot paritytech-review-bot bot requested a review from a team May 3, 2024 15:53
@jasl jasl force-pushed the align-crates-versions branch 2 times, most recently from e4a8859 to 8da46d4 Compare May 3, 2024 16:28
@bkchr bkchr requested a review from ggwpez May 3, 2024 20:55
@jasl
Copy link
Contributor Author

jasl commented May 3, 2024

Sorry, add a few more. This is enough for now

@svyatonik svyatonik removed the request for review from a team May 4, 2024 06:59
@paritytech-review-bot paritytech-review-bot bot requested a review from a team May 4, 2024 06:59
Copy link
Contributor

@liamaharon liamaharon left a comment

Choose a reason for hiding this comment

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

Any reason to not just switch to workspace deps?

@jasl
Copy link
Contributor Author

jasl commented May 4, 2024

Any reason to not just switch to workspace deps?

if you guys want, I can help to switch to workspace deps

@ggwpez
Copy link
Member

ggwpez commented May 4, 2024

Yea it would probably make more sense to pull them up first. Hope it can be automated like here: #3366

@jasl
Copy link
Contributor Author

jasl commented May 4, 2024

Yea it would probably make more sense to pull them up first. Hope it can be automated like here: #3366

Do you have any rules about which crates should be kept in the project's own Cargo.toml?

@liamaharon
Copy link
Contributor

Yea it would probably make more sense to pull them up first. Hope it can be automated like here: #3366

Do you have any rules about which crates should be kept in the project's own Cargo.toml?

Prob only if they would never be used in any other crate, I can't think of any examples.

@jasl
Copy link
Contributor Author

jasl commented May 5, 2024

Yea it would probably make more sense to pull them up first. Hope it can be automated like here: #3366

Do you have any rules about which crates should be kept in the project's own Cargo.toml?

Prob only if they would never be used in any other crate, I can't think of any examples.

Understood, tokio-tungstenite is just an example, it's only used in zombienet-backchannel

I'll do it later.

@ggwpez
Copy link
Member

ggwpez commented May 5, 2024

One thing to keep in mind that larger merge-requests normally take longer to review. We had a few sledge-hammer merge requests that ended up abandoned since the tried to do too much at once.
Its probably best to do it for a few deps per merge-request.

@jasl
Copy link
Contributor Author

jasl commented May 5, 2024

One thing to keep in mind that larger merge-requests normally take longer to review. We had a few sledge-hammer merge requests that ended up abandoned since the tried to do too much at once. Its probably best to do it for a few deps per merge-request.

Understood, I'll separate changes to small pieces

@jasl
Copy link
Contributor Author

jasl commented May 5, 2024

This should cover most, I'll split them tomorrow, and close this one

@jasl jasl mentioned this pull request May 5, 2024
@liamaharon
Copy link
Contributor

Understood, tokio-tungstenite is just an example, it's only used in zombienet-backchannel

I'd still make this a workspace dep, the dep is general and just because it isn't used by any other package today doesn't mean that won't change in the future

@liamaharon liamaharon added the R0-silent Changes should not be mentioned in any release notes label May 6, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 6, 2024
Split from #4374

This PR helps to reduce dependencies and align versions, which would
help to move them to workspace dep
@jasl jasl closed this May 6, 2024
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
Split from paritytech#4374

This PR helps to reduce dependencies and align versions, which would
help to move them to workspace dep
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
Split from paritytech#4374

This PR helps to reduce dependencies and align versions, which would
help to move them to workspace dep
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.

4 participants