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

Bump dependencies #140

Merged
merged 3 commits into from
Dec 11, 2024
Merged

Bump dependencies #140

merged 3 commits into from
Dec 11, 2024

Conversation

CleanCut
Copy link
Contributor

@CleanCut CleanCut commented Dec 11, 2024

Manual changes:

Changes to Cargo.lock from running cargo update:

@Copilot Copilot bot review requested due to automatic review settings December 11, 2024 20:52
@CleanCut CleanCut requested a review from a team as a code owner December 11, 2024 20:52

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (2)

crates/twirp/Cargo.toml:19

  • Ensure that there are adequate tests covering the changes introduced by updating the http crate from version 1.0 to 1.2, as this could introduce breaking changes or new behaviors.
http = "1.2"

crates/twirp/Cargo.toml:27

  • Ensure that there are adequate tests covering the changes introduced by updating the tokio crate from version 1.41 to 1.42, as this could introduce breaking changes or new behaviors.
tokio = { version = "1.42", default-features = false }

Tip: Turn on automatic Copilot reviews for this repository to get quick feedback on every pull request. Learn more

@CleanCut CleanCut self-assigned this Dec 11, 2024
@jorendorff
Copy link
Contributor

Update Rust from 1.77 to 1.83 (Is there any reason not to use the latest stable version of Rust?)

Bumping the Rust version effectively changes our MSRV. Although older versions would not instantly break, it definitely changes the minimum Rust version we test with.

Really we should have an explicit MSRV policy of some sort (even if it's "we only support the latest"), and set the rust-version property in crates/*/Cargo.toml accordingly. Would you mind giving that a shot?

Copy link
Contributor

@jorendorff jorendorff left a comment

Choose a reason for hiding this comment

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

Thanks! Approving conditionally:

The dependency updates alone are fine, if tests still pass with the old Rust version (that is, back out the version = 4 change in Cargo.lock and the rust-toolchain.toml change).

Or, document any reasonable MSRV policy in the README and set [toolchain] channel in rust-toolchain.toml and [package] rust-version in the Cargo.toml files accordingly. If the MSRV policy covers more than just the latest Rust release, ideally our CI should run the tests in both the oldest and the newest versions of Rust we claim to support.

rust-toolchain.toml Outdated Show resolved Hide resolved
@CleanCut CleanCut changed the title Update to Rust 1.83 and bump dependencies Bump dependencies Dec 11, 2024
@CleanCut
Copy link
Contributor Author

CleanCut commented Dec 11, 2024

I backed out the Rust version change and started a discussion about what MSRV policy to implement.

@CleanCut CleanCut added this pull request to the merge queue Dec 11, 2024
Merged via the queue into main with commit 71ceba8 Dec 11, 2024
2 checks passed
@CleanCut CleanCut deleted the bump-dependencies branch December 11, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants