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

Downgrade MSRV to 1.48 #85

Closed
wants to merge 1 commit into from
Closed

Downgrade MSRV to 1.48 #85

wants to merge 1 commit into from

Conversation

sdroege
Copy link
Contributor

@sdroege sdroege commented Nov 8, 2023

This is the version currently available in Debian oldstable and should be a
reasonable choice.

Also check in the CI that everything still compiles with that version
and fixed Cargo.lock files that use older dependency versions than what
cargo would select by itself.

@sdroege sdroege changed the title Downgrade MSRV to 1.47 and check on the CI Downgrade MSRV to 1.63 Nov 8, 2023
@sdroege sdroege marked this pull request as ready for review November 8, 2023 17:47
This is the version currently available in Debian oldstable and should be a
reasonable choice.

Also check in the CI that everything still compiles with that version
and fixed Cargo.lock files that use older dependency versions than what
cargo would select by itself.

fixup
@sdroege sdroege changed the title Downgrade MSRV to 1.63 Downgrade MSRV to 1.48 Nov 8, 2023
@sdroege
Copy link
Contributor Author

sdroege commented Nov 11, 2023

I'll try fixing the remaining CI issues tomorrow

@ia0 ia0 mentioned this pull request Nov 11, 2023
@ia0
Copy link
Owner

ia0 commented Nov 11, 2023

I'll try fixing the remaining CI issues tomorrow

I'm taking a look at it now in #87 because it's some annoying xtask shenanigans and possibly dealing with windows testing in github.

@ia0
Copy link
Owner

ia0 commented Nov 11, 2023

Ok, apparently powershell has aliases for cp, mv, and rm (although not exactly like on Linux), so it worked fine. If it's fine with you I'll merge #87 which is your commit and an xtask commit on top (also dealing with lints and updating the changelog). You can also equivalently cherry-pick the extra commit in this PR.

@sdroege
Copy link
Contributor Author

sdroege commented Nov 12, 2023

Let's merge yours then, that's already ready and doesn't need another CI run :)
You could've also pushed that commit directly into this PR btw, but doesn't really matter.

@sdroege
Copy link
Contributor Author

sdroege commented Nov 12, 2023

Also thanks for fixing up the CI :)

@ia0
Copy link
Owner

ia0 commented Nov 12, 2023

You could've also pushed that commit directly into this PR btw, but doesn't really matter.

I wasn't sure I had permission to push to your branch. But it doesn't change anything AFAIU (squashed PRs with multiple authors are authored by the merger and co-authored by the rest).

Also thanks for fixing up the CI :)

No worries, that was on me anyway. And thanks again for bringing this issue to my attention! And I'm actually surprised you discovered it because the MSRV bump was not even released, which means you had to take a look at the repo.

@ia0 ia0 closed this Nov 12, 2023
@sdroege sdroege deleted the msrv-1.47 branch November 12, 2023 12:09
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