Skip to content

Use dep: and optional dependency feature ? syntax#1608

Merged
dtolnay merged 4 commits intodtolnay:masterfrom
BD103:deps-features
Mar 30, 2024
Merged

Use dep: and optional dependency feature ? syntax#1608
dtolnay merged 4 commits intodtolnay:masterfrom
BD103:deps-features

Conversation

@BD103
Copy link
Contributor

@BD103 BD103 commented Mar 30, 2024

This PR comes in 2 parts:

The first uses the dep: syntax for the printing feature depending on quote, which removes the unused implicit feature.

The second uses the ? syntax in the proc-macro feature, so quote is not pulled in as a dependency if printing is not enabled.

Unfortunately, both of these changes raise the MSRV back to 1.60. Before finishing my changes in CI, I wanted to confirm that this was fine.

BD103 added 3 commits March 29, 2024 22:06
This syntax needs Rust 1.60, so it will require a MSRV bump. If this is not wanted, I would be willing to revert it.
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.60 is fine. Thanks!

This change is using 6e8a372 for reference. I'm not entirely sure what the purpost of `manifestpath` was, so I may have done something incorrect.
@dtolnay dtolnay merged commit 585df47 into dtolnay:master Mar 30, 2024
@BD103 BD103 deleted the deps-features branch March 30, 2024 02:47
@BD103
Copy link
Contributor Author

BD103 commented Mar 30, 2024

Thank you! (That was fast 😄)

@dtolnay
Copy link
Owner

dtolnay commented Mar 30, 2024

Published in 2.0.57.

@joshlf
Copy link

joshlf commented Apr 2, 2024

Just a heads up that the MSRV bump broke zerocopy: google/zerocopy#1085. I know that there's no ecosystem consensus on whether MSRV bumps should be considered breaking changes; just wanted to provide a data point.

If syn's MSRV policy is to not consider MSRV bumps to be breaking changes, it'd be great if that were documented somewhere so we'd know to be aware of the risk.

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