search

servo / rust-url

URL parser for Rust
https://docs.rs/url/
Apache License 2.0
1.27k stars 318 forks source link

Update MSRV to 1.60.0 #849

Closed lucacasonato closed 1 year ago

lucacasonato commented 1 year ago

This fixes CI.

lucacasonato commented 1 year ago

cc @valenting because I seem to have lost access to the repo. Can't use "Request reviews" anymore 🙃

lucacasonato commented 1 year ago

We haven't specified an MSRV in Cargo.toml previously. I'll add that now. Scratch that, we added it recently. Updating now.

We bumped from 1.35 to 1.45 to 1.51 to 1.56 over the last two years without issue, so I don't expect this one to be a problem either. 1.60 is over a year old, so it's probably OK at this point. For a point of reference, the very widely used log crate has MSRV >= 1.60 now.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +0.02 :tada:

Comparison is base (0e25146) 82.44% compared to head (d2e79da) 82.46%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #849 +/- ## ========================================== + Coverage 82.44% 82.46% +0.02% ========================================== Files 20 20 Lines 3343 3348 +5 ========================================== + Hits 2756 2761 +5 Misses 587 587 ``` | [Impacted Files](https://app.codecov.io/gh/servo/rust-url/pull/849?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo) | Coverage Δ | | |---|---|---| | [url/src/lib.rs](https://app.codecov.io/gh/servo/rust-url/pull/849?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo#diff-dXJsL3NyYy9saWIucnM=) | `75.96% <ø> (-0.11%)` | :arrow_down: | ... and [5 files with indirect coverage changes](https://app.codecov.io/gh/servo/rust-url/pull/849/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=servo)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Manishearth commented 1 year ago

Yep, 1.60 is quite conservative anyway, but I wasn't comfortable approving this myself :smile:

Given the past cadence, I am now

valenting commented 1 year ago

If I'm not mistaken the MSRV mismatch is caused by the debugger_test dev-dependencies (which eventually depend on log) which are actually only tested/needed on nightly for debugger_visualizer.

If that's the case, maybe we can move the dev-dependency and tests to a subcrate, and add a special CI task for it. That way we can avoid updating the MSRV just yet and make sure and make those dependencies actually nighly only?

valenting commented 1 year ago

I think we can close this for now. @lucacasonato please reopen if you still think we should up the MSRV.