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

Update bstr to 1.x #40

Closed
wants to merge 1 commit into from
Closed

Update bstr to 1.x #40

wants to merge 1 commit into from

Conversation

dbanty
Copy link

@dbanty dbanty commented Sep 25, 2022

The only impacting breaking change seems to be that enabling unicode support requires enabling std.

@mitsuhiko
Copy link
Owner

Need to figure out why this does not run CI. Created a second PR for this here to get CI to run: #41

This likely will mean that bstr support will have to move the MSRV up significantly for when bstr is used.

@mitsuhiko
Copy link
Owner

Looks like I won't be able to merge this for a while. I am using the bstr feature in insta via similar still and I want to at least try to support older versions of rustc for quite a while longer since it's a testing tool. Not sure yet how to deal with this.

@dbanty
Copy link
Author

dbanty commented Sep 25, 2022

Gotcha, no worries, I was just trying to remove a duplicate dependency from something else that consumes this. Thanks for figuring that out!

@mitsuhiko
Copy link
Owner

Yeah, that makes a lot of sense. I'm going to figure out what to do here. Made a meta issue over here: mitsuhiko/insta#289

@BurntSushi
Copy link

The only impacting breaking change seems to be that enabling unicode support requires enabling std.

Just a note that, previously, I believe enabling unicode implicitly enabled std via lazy_static. bstr 1.0 was going to do the same with once_cell, but then I realized it would be a backcompat hazard. So I required folks to be explicit about it. That way, I can remove std as a dependency of the unicode feature in the future. (And I do expect this to happen at some point by dropping once_cell.)

Here's the commit where I made the change that has some more details: BurntSushi/bstr@72024a8

@smoelius
Copy link

I would like to respectfully bump this.

As mentioned in mitsuhiko/insta#289, merging this PR would require similar's MSRV to be atleast 1.60 (April 2022).

Do you know of similar dependents that require a compiler that old? If not, would it be possible to move forward with this?

@mitsuhiko
Copy link
Owner

Insta (and thus similar) is used by libraries that target 1.56. syn being an example.

@smoelius
Copy link

smoelius commented Mar 20, 2024

Should that matter, though, since insta is only a dev-dependency of syn?

EDIT: FWIW, I ran this in the syn repository:

cargo msrv -- cargo check --tests --all-features

And the result was 1.62.1.

@mitsuhiko mitsuhiko mentioned this pull request Mar 28, 2024
@mitsuhiko mitsuhiko closed this Mar 28, 2024
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.

4 participants