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

ci(github): fix path filtering of required but skipped workflows #2977

Conversation

petermetz
Copy link
Contributor

@petermetz petermetz commented Jan 15, 2024

  1. Migrated from the stock GitHub Actions yaml syntax to using this custom
    action[1] that allows us to skip the running of jobs based on path filtering.
    The difference compared to the old solution is that this one won't just
    skip the checks that are marked as required but it will pass them instead.
  2. This is a suboptimal solution because we have to run each job so that
    the code can be checked out and then examined for changes. This uses
    resources that we should not have to expend, but currently there's no way
    to avoid this because of the way the GitHub handles required checks that
    are skipped (which cause a deadlock on the CI at the currently).
  3. There is a relevant discussion thread about the problem here [2] which
    should be voted on heavily so that it gets the necessary attention from
    the product teams at GitHub.

Quoting the relevant part of the GitHub documentation directly:

Handling skipped but required checks
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."

Source (link broken into multiple lines to avoid it being longer than
100 characters):

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/
collaborating-on-repositories-with-code-quality-features/
troubleshooting-required-status-checks#handling-skipped-but-required-checks

[1] https://github.com/dorny/paths-filter
[2] https://github.com/orgs/community/discussions/44490

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

@petermetz petermetz force-pushed the ci-fix-path-filtering-required-but-skipped-checks branch from 76d2414 to 5bcd009 Compare January 17, 2024 03:55
@sandeepnRES sandeepnRES force-pushed the ci-fix-path-filtering-required-but-skipped-checks branch from 8ae311e to 7fb8d8f Compare January 17, 2024 14:18
1. Migrated from the stock GitHub Actions yaml syntax to using this custom
action[1] that allows us to skip the running of jobs based on path filtering.
The difference compared to the old solution is that this one won't just
skip the checks that are marked as required but it will pass them instead.
2. This is a suboptimal solution because we have to run each job so that
the code can be checked out and then examined for changes. This uses
resources that we should not have to expend, but currently there's no way
to avoid this because of the way the GitHub handles required checks that
are skipped (which cause a deadlock on the CI at the currently).
3. There is a relevant discussion thread about the problem here [2] which
should be voted on heavily so that it gets the necessary attention from
the product teams at GitHub.

Quoting the relevant part of the GitHub documentation directly:

> Handling skipped but required checks
> 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."

**Source** (link broken into multiple lines to avoid it being longer than
100 characters):

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/
collaborating-on-repositories-with-code-quality-features/
troubleshooting-required-status-checks#handling-skipped-but-required-checks

[1] https://github.com/dorny/paths-filter
[2] https://github.com/orgs/community/discussions/44490

Signed-off-by: Peter Somogyvari <[email protected]>
Signed-off-by: Sandeep Nishad <[email protected]>
@sandeepnRES sandeepnRES force-pushed the ci-fix-path-filtering-required-but-skipped-checks branch from 7fb8d8f to 6465e93 Compare January 17, 2024 15:20
@petermetz petermetz merged commit f9d1c92 into hyperledger-cacti:main Jan 17, 2024
109 of 140 checks passed
@petermetz
Copy link
Contributor Author

@sandeepnRES Thank you very much for the help and the optimizations!

@petermetz petermetz deleted the ci-fix-path-filtering-required-but-skipped-checks branch January 17, 2024 23:48
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.

3 participants