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: use dorny/paths-filter to include/skip a job with a required status. #651

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

ericswanson-dfinity
Copy link
Member

@ericswanson-dfinity ericswanson-dfinity commented Jan 26, 2024

Overview

I can't merge #645, because GitHub says I am missing a required status. What status that might be isn't obvious.

GitHub's docs say:

Warning: If a workflow is skipped due to path filtering, branch filtering or a commit message, then checks associated with that workflow will remain in a "Pending" state. A pull request that requires those checks to be successful will be blocked from merging.

For this reason you should not use path or branch filtering to skip workflow runs if the workflow is required. For more information, see "Skipping workflow runs" and "Required workflows."

If, however, a job within a workflow is skipped due to a conditional, it will report its status as "Success". For more information, see "Using conditions to control job execution."

It turns out that "paths-ignore" doesn't solve the problem. See https://github.com/orgs/community/discussions/13690

This PR takes two jobs that report a required status, and skips them with a conditional. If this works out (can't test it against the real statuses until it's merged to master), I'll copy this to the other workflows that generate required statuses.

Here is the workflow working on this PR: https://github.com/dfinity/examples/actions/runs/7674715486/job/20919794867?pr=651

Here is the workflow skipping both jobs in another PR: https://github.com/dfinity/examples/actions/runs/7674641514/job/20919580001?pr=650

Considered Solutions
I considered running the required jobs unconditionally, which would have been simpler, but also would have run tests unnecessarily.

Considerations
This will require a runner to evaluate the conditional, but it doesn't have to check out the repo.

@ericswanson-dfinity ericswanson-dfinity marked this pull request as ready for review January 27, 2024 00:05
@ericswanson-dfinity ericswanson-dfinity requested a review from a team as a code owner January 27, 2024 00:05
@ericswanson-dfinity ericswanson-dfinity merged commit 7074748 into master Jan 29, 2024
78 checks passed
@ericswanson-dfinity ericswanson-dfinity deleted the required-statuses branch January 29, 2024 14:10
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.

2 participants