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

core-foundation: Use dep syntax for with-uuid feature. #691

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

waywardmonkeys
Copy link
Contributor

@waywardmonkeys waywardmonkeys commented Jul 20, 2024

This removes an implicit feature due to the optional dependency uuid.

While this is user visible, any use of uuid as a feature previously would not have enabled the support code and so it wouldn't have had any benefit.

Bump the MSRV in CI to 1.64 (while only 1.60 is required here, other features from 1.64 will be used).

@waywardmonkeys
Copy link
Contributor Author

Dep syntax needs MSRV 1.60 or later.

@mrobinson
Copy link
Member

You'll also need to update the MSRV in this change in the Cargo.toml (if it isn't already defined) and fix the CI job.

@waywardmonkeys
Copy link
Contributor Author

@mrobinson Any preference what it gets updated to?

@mrobinson
Copy link
Member

@mrobinson Any preference what it gets updated to?

I suppose we should shoot for the oldest version possible.

@waywardmonkeys waywardmonkeys force-pushed the use-explicit-dep-for-uuid branch from cbb7f76 to 2d1db9c Compare July 23, 2024 18:38
@waywardmonkeys
Copy link
Contributor Author

@mrobinson For now, I only bumped it in the CI job. I'd rather do a full separate PR that fixes up the package info to include the rust-version separately if that's okay.

@waywardmonkeys
Copy link
Contributor Author

Also, I've bumped to 1.64 rather than 1.60 as that'll get us the features for #692 and also workspace dependencies and package settings.

@waywardmonkeys
Copy link
Contributor Author

The most common dependency of this crate is security-framework and I asked over there about their MSRV as they claim 1.60, but actually test for 1.65:

kornelski/rust-security-framework#208

@mrobinson
Copy link
Member

Okay. 1.65 seems reasonable.

@waywardmonkeys
Copy link
Contributor Author

@mrobinson Would've been nice to have gotten a heads up that a new version was going to be published, including a breaking semver and we could've gotten this landed ... Not sure why I bother with any of these PRs.

@waywardmonkeys waywardmonkeys mentioned this pull request Aug 11, 2024
This removes an implicit feature due to the optional dependency
`uuid`.

While this is user visible, any use of `uuid` as a feature previously
would not have enabled the support code and so it wouldn't have had
any benefit.
@waywardmonkeys waywardmonkeys force-pushed the use-explicit-dep-for-uuid branch from 2d1db9c to ab88b21 Compare August 11, 2024 17:44
@waywardmonkeys
Copy link
Contributor Author

@jdm @mrobinson This too is good to go for review.

@waywardmonkeys waywardmonkeys force-pushed the use-explicit-dep-for-uuid branch from 9c6db75 to ab88b21 Compare August 11, 2024 17:49
@jdm jdm added this pull request to the merge queue Aug 11, 2024
Merged via the queue into servo:main with commit b10b72c Aug 11, 2024
14 checks passed
@waywardmonkeys waywardmonkeys deleted the use-explicit-dep-for-uuid branch August 12, 2024 00:43
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.

3 participants