Skip to content

Conversation

@david-mcgillicuddy-moixa
Copy link

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

Replace use of str::split_once, str::rsplit_once, and concat! inside doc comments, to enable 1.51 compat

PR Type

Other

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

Hi, as previously discussed in actix/actix-net#434 we here at Moixa are stuck on 1.51.0 and currently stuck on old versions of the various actix beta crates. This fork (and PR) reverts the MSRV to 1.51.0, and not before that due to use of cargo's resolver feature.

I haven't quite finished the supporting changes such as the changelog, CI, and label changes since this PR is more substantial than the actix-net one and impacts the crate more significantly (but still invisibly from the outside) with the doc comments being more cumbersome to write and edit so I wanted to see what you thought about upstreaming it here first before doing that stuff.

I have generated the documentation and couldn't see any changes at all as I'd hope, and the new functions are already covered by the existing tests.

@robjtede
Copy link
Member

robjtede commented Jan 6, 2022

I've realised that this might not be possible due our dependency on cookie which is MSRV 1.53 and rustls which is MSRV 1.52.1.

To be honest, this is starting to look like more of a pain for maintenance that I would like.

@robjtede robjtede added A-http project: actix-http A-web project: actix-web B-semver-patch labels Jan 6, 2022
@david-mcgillicuddy-moixa
Copy link
Author

david-mcgillicuddy-moixa commented Jan 7, 2022

Yes I agree on reflection, that doc comment change turns some awkward code into something more unpleasant. Okay forget this actix-web one then, although as said I'll continue to maintain the fork (and possibly fork those libraries too) at least while Moixa hasn't upgraded its build just to let anybody know who stumbles across this.

The actix-net one seems much more reasonable to me though so I would like to continue discussing that one if you're amenable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-http project: actix-http A-web project: actix-web B-semver-patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants