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

feat: Support mixed MSRV in --version-range #213

Merged
merged 6 commits into from
Sep 9, 2023
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Sep 8, 2023

This expands on the approach taken in #212 bucketing packages into
rust-versions to run. If we skipped the MSRV (due to --version-step),
we automatically inject it. If a package's MSRV isn't within the range,
we skip it.

Benefits

  • Relatively simple to implement and to explain
  • We keep the number of runs to a minimum by walking in lock-step the
    --version-step, independent of what each package' MSRV

I did have to specialize --rust-version vs --version-range to avoid
--rust-version range users walking more than they needed.

To keep the progress total accurate, I shifted the calculating of the
total from determine_package_list to after we have bucketed
everything. To make this feasible, I saved off the how many iterations
a package will have without the version range being taken into account.

As a byproduct, this fixes a bug in #212 where it didn't take the
rust-version into account when determining the total.

Fixes #199

This expands on the approach taken in taiki-e#212 bucketing packages into
rust-versions to run.  If we skipped the MSRV (due to `--version-step`),
we automatically inject it.  If a package's MSRV isn't within the range,
we skip it.

Benefits
- Relatively simple to implement and to explain
- We keep the number of runs to a minimum by walking in lock-step the
  `--version-step`, independent of what each package' MSRV

I did have to specialize `--rust-version` vs `--version-range` to avoid
`--rust-version` range users walking more than they needed.

To keep the progress total accurate, I shifted the calculating of the
total from `determine_package_list` to after we have bucketed
everything.  To make this feasible, I saved off the how many iterations
a package will have without the version range being taken into account.

As a byproduct, this fixes a bug in taiki-e#212 where it didn't take the
rust-version into account when determining the total.

Fixes taiki-e#199
@epage
Copy link
Contributor Author

epage commented Sep 8, 2023

@taiki-e CI is passing

@taiki-e
Copy link
Owner

taiki-e commented Sep 8, 2023

Great! Could you add a test case for --version-step?

@epage
Copy link
Contributor Author

epage commented Sep 8, 2023

I also should add a warning when a selected package is skipped due to msrv being higher than the end of the range in case it isn't intentional

@taiki-e taiki-e added the A-version Area: --rust-version, --version-range, --version-step label Sep 8, 2023
@epage
Copy link
Contributor Author

epage commented Sep 8, 2023

CI is passing again with the applied changes

Copy link
Owner

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks!

@taiki-e taiki-e merged commit 6b607d3 into taiki-e:main Sep 9, 2023
24 checks passed
@taiki-e
Copy link
Owner

taiki-e commented Sep 9, 2023

Published in 0.6.6. Thanks again @epage!

@epage epage deleted the run branch September 11, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-version Area: --rust-version, --version-range, --version-step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

version-range: Support multiple different MSRVs in a workspace
2 participants