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 deps and switch to 2021 edition #715

Merged
merged 2 commits into from Oct 5, 2022
Merged

Update deps and switch to 2021 edition #715

merged 2 commits into from Oct 5, 2022

Conversation

lwandrebeck
Copy link
Contributor

cargo bootstrap and build run fine.
Update to 2021 edition.
Update to latest ucd-trie, serde, serde_json, thiserror.

@lwandrebeck lwandrebeck requested a review from a team as a code owner October 4, 2022 08:58
@lwandrebeck lwandrebeck requested review from NoahTheDuke and removed request for a team October 4, 2022 08:58
@lwandrebeck lwandrebeck changed the title Update deps and swith to 2021 edition Update deps and switch to 2021 edition Oct 4, 2022
@lwandrebeck
Copy link
Contributor Author

Some errors do appear here: https://github.com/pest-parser/pest/actions/runs/3180773049/jobs/5184774281 even though test pass properly. Digging into it.

Copy link
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

lgtm; I tried to run cargo fix --edition https://doc.rust-lang.org/edition-guide/editions/transitioning-an-existing-project-to-a-new-edition.html and no files were changed, so this should be fine as it is.

The semver error looks like the one I mentioned in the third point here:
#705 (comment) because those reported violations on pest_meta look to be false positives.

@lwandrebeck
Copy link
Contributor Author

@tomtau : I’ve ran it and got no change too. But, panic! is supposed not to be used as-is in 2021 edition. Maybe something slipped through the cracks between documentation and actual code ?

@NoahTheDuke
Copy link
Member

I don't remember all of the details about editions; will this have any effects on MSRV?

@lwandrebeck
Copy link
Contributor Author

1.56 brings 2021 edition, and we’re at 1.56.1 according to README.md
https://blog.rust-lang.org/2021/10/21/Rust-1.56.0.html

@NoahTheDuke
Copy link
Member

Oh thanks for digging that out, excellent. Then yeah, this is good to go.

@tomtau
Copy link
Contributor

tomtau commented Oct 4, 2022

There are some cases where the automated fixes are not applied: https://doc.rust-lang.org/edition-guide/editions/advanced-migrations.html#migrating-macros (macros, features, generated code, plus some fixes are optional, so it needs the --edition-idioms flag).
@lwandrebeck are you sure about the panic fix? My understanding of https://doc.rust-lang.org/edition-guide/rust-2021/panic-macro-consistency.html is that 2021 makes panic! usage to be consistent with println!, and that original line was consistent with it (hence there wasn't any automated fix)?

@tomtau
Copy link
Contributor

tomtau commented Oct 4, 2022

For the context, previously MSVR was lower than 1.56, hence the edition was only up to 2018: #615 (comment) (but it was later increased to 1.56: #654 )

@lwandrebeck
Copy link
Contributor Author

@tomtau : I’ve never put my hands at all into macros to be fair, so that’s out of my league. I’ve just re-run cargo fix --edition --edition-idioms --allow-dirty after putting back 2018 in Cargo.toml files without getting any change once again.
I’ll try to dig a bit more into that subject, but that won’t be until next day at best. If anyone is willing to check that point, go ahead :)

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.

None yet

3 participants