Skip to content

Conversation

david-mcgillicuddy-moixa
Copy link
Contributor

@david-mcgillicuddy-moixa david-mcgillicuddy-moixa commented Jan 6, 2022

PR Type

Bug Fix I think?

PR Checklist

Check your PR fulfills the following:

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt

Overview

This PR replaces the use of str::split_once, which was stablised in 1.52.0 with str::splitn, keeping the MSRV at 1.50.0. This still isn't the 1.46.0 currently mentioned in the readme, due to the use of bool_to_option at actix-server/src/server.rs:211 but I haven't gone that far back, I can do if preferred.

I could also gate this behind a conditional compilation directive if preferred so that users not stuck 6+ months in the past can use the new methods but also it's just a small string helper method so I don't believe that the additional complexity is worth it myself.

While this lowers the MSRV of actix-tls from 1.52.0 to 1.50.0 it raises the MSRV for the rest of the repo from 1.46.0 to 1.50.0 (due to no longer having a bunch of exclusions to the MSRV). I can revert that and keep everything on 1.46.0 if preferred. This way actix-tls doesn't have to be excluded from the MSRV requirement. Or I could just leave actix-server excluded and the rest of the repo can stay on 1.46.0 I guess.

Closes #433

@david-mcgillicuddy-moixa david-mcgillicuddy-moixa changed the title Replace str_split_once to lower msrv to 1.50.0 Replace str_split_once to lower actix-tls msrv to 1.50.0 and bump actix-net to 1.50.0 Jan 6, 2022
@robjtede
Copy link
Member

robjtede commented Jan 9, 2022

Can you put all the CI changes back please. I will accept this PR but don't want to change the official MSRV in CI even if it will build on the earlier compiler. That's to say, I still recommend trying to update your toolchain.

@david-mcgillicuddy-moixa
Copy link
Contributor Author

Done now. And yes we very much still are it's just unfortunately a longer-term proposition.

@robjtede robjtede merged commit 9ec3cc0 into actix:master Jan 10, 2022
@david-mcgillicuddy-moixa
Copy link
Contributor Author

Great thanks

@robjtede
Copy link
Member

released this in actix-tls 3.0.1

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.

actix-tls v3 uses str_split_once and bumps MSRV to 1.52

2 participants