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

fix: migrated to 2018 edition #615

Merged
merged 1 commit into from Jul 10, 2022
Merged

Conversation

tomtau
Copy link
Contributor

@tomtau tomtau commented Jul 10, 2022

closes #300

the code was migrated via cargo fix --edition
and the grammar benchmarks were rewritten to use bencher
instead of the nightly test (so that they can be built
on a stable version).

@tomtau tomtau requested a review from CAD97 July 10, 2022 06:45
@tomtau tomtau changed the title fix: migrated to 2021 edition fix: migrated to 2018 edition Jul 10, 2022
closes pest-parser#300

the code was migrated via `cargo fix --edition`
and the grammar benchmarks were rewritten to use `bencher`
instead of the nightly test (so that they can be built
on a stable version).
@glyn
Copy link

glyn commented Jul 10, 2022

What is your rationale for migrating to the 2018 edition rather than 2021?

@tomtau
Copy link
Contributor Author

tomtau commented Jul 10, 2022

@glyn I was originally planning 2021, but it seems the current minimal version on CI doesn't support it: https://github.com/pest-parser/pest/runs/7268848504?check_suite_focus=true hence I switched it to 2018.

maybe good for @CAD97 to comment whether it's ok to bump up MSRV.

@birkenfeld
Copy link
Contributor

IMO: don't bump MSRV for such a minor thing, the 2021 edition probably doesn't have anything to offer pest.

@CAD97
Copy link
Contributor

CAD97 commented Jul 10, 2022

If we bump MSRV it should be in a separate patch. MSRV should ideally only be increased for a specific benefit; I think it's reasonable to set an MSRV of 1.56 for the rust-version manifest key. 1.56 will be a year old on October 21; stable-6 is a common community definition of LTS.

For reference:

distro version release date
debian oldstable 1.41 Jan 30 2020
debian stable 1.48 Nov 19 2020
debian nextstable 1.59 Feb 24 2022
debian unstable 1.59 Feb 24 2022
tokio (tested) 1.49 Dec 31 2020
tokio (guaranteed) stable-6 (1.56) Oct 21 2021

Copy link
Contributor

@CAD97 CAD97 left a comment

Choose a reason for hiding this comment

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

Personally I strongly prefer using criterion, but the choice to use bencher here for a smaller diff is reasonable.

Diff is the mechanical edition update, as described.

pest/src/span.rs Show resolved Hide resolved
@CAD97 CAD97 merged commit 62fcef3 into pest-parser:master Jul 10, 2022
@tomtau
Copy link
Contributor Author

tomtau commented Jul 11, 2022

Thanks @CAD97 . I originally used criterion, but it failed on CI due to one of its dependencies requiring edition 2021, hence bencher.
I guess if MSRV is bumped to stable-6, it should be ok to switch to criterion.

@CAD97
Copy link
Contributor

CAD97 commented Jul 11, 2022

It should be fine to only run benchmarks/tests/examples on current stable rather than MSRV, so it shouldn't be a problem for them to have a higher MSRV.

@tomtau tomtau deleted the fix/edition-2021 branch July 11, 2022 00:40
tomtau pushed a commit to tomtau/pest that referenced this pull request Jul 15, 2022
as per pest-parser#615 (comment)
besides criterion, it is now possible to compile the
`const_prec_climber` feature on stable, because
`const_fn_trait_bound` was stabilized
tomtau pushed a commit to tomtau/pest that referenced this pull request Jul 15, 2022
as per pest-parser#615 (comment)
besides criterion, it is now possible to compile the
`const_prec_climber` feature on stable, because
`const_fn_trait_bound` was stabilized
plus clippy fixes
tomtau pushed a commit to tomtau/pest that referenced this pull request Jul 15, 2022
as per pest-parser#615 (comment)
besides criterion, it is now possible to compile the
`const_prec_climber` feature on stable, because
`const_fn_trait_bound` was stabilized
plus clippy fixes
tomtau pushed a commit to tomtau/pest that referenced this pull request Jul 15, 2022
as per pest-parser#615 (comment)
besides criterion, it is now possible to compile the
`const_prec_climber` feature on stable, because
`const_fn_trait_bound` was stabilized
plus clippy fixes
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.

Migrate to 2018 edition
4 participants