Move MSRV jobs into main ci.yml
workflow
#2027
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves the
check-msrv
andcheck-msrv-badge
job definitions frommsrv.yml
toci.yml
, where they are renamedmsrv
andmsrv-badge
, respectively.This includes those jobs among the PR-blocking checks in
ci.yml
(i.e.,tests-pass
depends on them). This lets failure ofmsrv
continue blocking PR auto-merge (check-msrv
had done so), and causesmsrv-badge
to begin blocking PRs (check-msrv-badge
had not done so).The
msrv.yml
workflow is accordingly removed. See #2026 for relevant context and discussion. This should only be merged if I am correct to think that these no longer benefit from being separate workflows.If this is merged, then branch protection rules should be adjusted to remove the required MSRV checks now that they are covered by
tests-pass
inci.yml
. However, it looks like nothing will break in between merging this and removing those required checks: moving a job from one workflow to another, and even renaming it in the sense of changing its job ID (the YAML key that introduces it), does not break a required check for the job, so long as it had aname
key that was unchanged!One integration subtlety here is that, if those required checks are removed, then preexisting PRs could potentially auto-merge even if they fail MSRV checks. This could be addressed by manually verifying that they pass the checks; or by rebasing them onto main; or by waiting to remove the old required MSRV checks until any preexisting PRs are rebased onto main, merged, or closed; or by not worrying about it if no MSRV-related changes are expected in any of them.
This rests right on the line between what I would readily merge without a review and what I would wait for a review on. The only disadvantage I can think of here is if running MSRV checks, and no others, through
workflow-dispatch
, is desired. I don't think that's an important thing to be able to do, but if it is then themsrv.yml
workflow should be kept and this PR should not be merged.The change here is fairly non-disruptive and could be reverted... but the resulting commit history would be somewhat harder to understand. Since this doesn't seem to be blocking anything--in particular, I don't have any further changes to make to the details of the MSRV jobs at this time--I think I'll wait for a review.