-
Notifications
You must be signed in to change notification settings - Fork 182
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 syn to 2.0 #3318
Update syn to 2.0 #3318
Conversation
86167a2
to
2318d5a
Compare
2318d5a
to
59fe613
Compare
59fe613
to
9385e3c
Compare
Ready for review. I do not think it is necessary to do patch releases with this; I am posting this PR now precisely because I think the time for patch releases is (mostly) past. |
Hm, displaydoc needs a syn update too. Might do this later. |
ah, there's an open PR: yaahc/displaydoc#42 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-request review once CI is green
a6503d0
to
9dda80d
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
(removed an unnecessary lockfile change that was a separate commit) |
fn parse(input: ParseStream<'_>) -> parse::Result<Self> { | ||
let path: Path = input.parse()?; | ||
|
||
fn at_most_one_option<T>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Observation: it seems the old code didn't actually care about this; it just took whichever option was the last one parsed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it's better to have a little bit more validation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically it's a breaking change but 🤷
a1d66ac
to
92ff420
Compare
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
kicking CI, since it's failing without logs and the tests pass locally. Force push did nothing but update commit date of the last commit |
fn parse(input: ParseStream<'_>) -> parse::Result<Self> { | ||
let path: Path = input.parse()?; | ||
|
||
fn at_most_one_option<T>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically it's a breaking change but 🤷
This updates syn to 2.0 for all internal ICU4X crates/utils.
This does not update Diplomat, that is WIP in rust-diplomat/diplomat#324. I might try to land that first so I can include it as a git dep update here (before doing a proper release)Should only merge after the 1.2 release (resulting in patch releases for the dependencies)