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

Explain how to handle skipped required checks #8926

Closed
1 task done
Tracked by #26845
jsoref opened this issue Aug 10, 2021 · 19 comments · Fixed by #9126
Closed
1 task done
Tracked by #26845

Explain how to handle skipped required checks #8926

jsoref opened this issue Aug 10, 2021 · 19 comments · Fixed by #9126
Labels
content This issue or pull request belongs to the Docs Content team help wanted Anyone is welcome to open a pull request to fix this issue

Comments

@jsoref
Copy link
Contributor

jsoref commented Aug 10, 2021

Code of Conduct

  • I have read and agree to the GitHub Docs project's Code of Conduct.

What article on docs.github.com is affected?

https://github.com/github/docs/blob/2289ca70c7ec7ec1b81eafdb333cd0157fafaa08/content/github/administering-a-repository/troubleshooting-required-status-checks.md

What part(s) of the article would you like to see updated?

A section should be added that explains that if people use paths for a required check, they'll want to add a second copy of the check with paths-ignore for the same content that just returns true.

Additional information

It's a FAQ in https://github.community

@jsoref jsoref added the content This issue or pull request belongs to the Docs Content team label Aug 10, 2021
@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Aug 10, 2021
@Fadiabukhadrah

This comment has been minimized.

@ramyaparimi ramyaparimi removed the triage Do not begin working on this issue until triaged by the team label Aug 11, 2021
@lecoursen
Copy link
Member

It's a FAQ in https://github.community

Hi @jsoref, could you link to one or more examples? Thanks!

@lecoursen
Copy link
Member

Thanks @jsoref! This sounds like a great plan that will address a very common source of confusion. You or anyone else is welcome to open that PR! ⚡

@lecoursen lecoursen added the help wanted Anyone is welcome to open a pull request to fix this issue label Aug 17, 2021
@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Oct 17, 2021
@ramyaparimi ramyaparimi removed the stale There is no recent activity on this issue or pull request label Oct 18, 2021
hubwriter added a commit that referenced this issue Nov 29, 2021
* Update troubleshooting-required-status-checks.md

* Add troubleshooting info

Adds how to handle skipped required checks.

Fixes #8926

* Fix generic workflow

* Add a note about other CI/CD

* Add images

* Added images

* Delete troubleshooting-required-status-checks.md

* Fix merge conflicts

* Remove blank lines

Co-authored-by: hubwriter <hubwriter@github.com>

* Apply styling

* Group notes into one

* Remove older images

* Remove images

* Added suggested images

* Update content/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks.md

Co-authored-by: hubwriter <hubwriter@github.com>
@Fran-Rg
Copy link

Fran-Rg commented Aug 24, 2022

Because I landed here from google search, the documented solution is here: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks

@TWiStErRob
Copy link
Contributor

@jsoref I'm trying to research this topic, but all the links in #8926 (comment) are dead. Where can I find those old forum topics?

@jsoref
Copy link
Contributor Author

jsoref commented Nov 6, 2022

Dunno. They did a real number in github.community in their migration. It didn't occur to me when I filed this that I'd need to quote the full contents of each item here...

@TWiStErRob
Copy link
Contributor

I tried searching for some slugs without dashes but no results, not even on Google.

@jsoref
Copy link
Contributor Author

jsoref commented Nov 6, 2022

@TWiStErRob
Copy link
Contributor

Oo nice, forgot about that.

@cb-shivamagarwal
Copy link

cb-shivamagarwal commented Feb 28, 2023

May someone please review and merge - #24182 ?

In the section handling-skipped-but-required-checks, the docs do not shed any light for the case mentioned below:

When someone sends a pull request that changes the files listed under paths and also changes a file that is not listed under paths-ignore in the other workflow. In such a case, both workflows must be executed and the associated status checks must pass in order to merge the PR.

This PR adds documentation for that

@minherz
Copy link

minherz commented Mar 12, 2023

The proposed solution is confusing because it causes the check list to display the same workflow / job names twice when changes in a pull request fit both filters (e.g. paths and paths-ignore).
So far the solution looks more like a problem mitigation. It would be nice having a smarter check list execution logic for required checks. Is it possible to mark the execution is completed when the check is skipped due to filtering?

@jsoref
Copy link
Contributor Author

jsoref commented Mar 12, 2023

Sadly, this repository is focused on documentation. For technical implementation details, there are two main paths that externals (like us) can take:

I sympathize with your concerns, but it's much easier for me to get documentation added than to get implementation changed.

@JJ-connect
Copy link

The proposed solution has been removed (as of this commit).

What is now the recommended way of dealing with this?

The docs seem to imply that using a conditional rather than path/branch filtering is the correct way to deal with this.
If this is recommended, could there be an example to show how it works?

PS: does anyone have a link for this issue (as a technical implementation) to follow up there too?

@jsoref
Copy link
Contributor Author

jsoref commented Jul 14, 2023

When a workflow is run as a required workflow it will ignore all the filters in the on: section, for example: branches, branches-ignore, paths, types etc. The required workflow will run only for the pull_request and pull_request_target default events. For more information on default activity types, see "Events that trigger workflows."

https://docs.github.com/en/actions/using-workflows/required-workflows#prerequisites

@minherz
Copy link

minherz commented Jul 17, 2023

Great! If previously it was inconvenient, now it became a nonsense.

There are two problems with the document (hence, implicitly, implementation of the "required check" feature) that @jsoref referenced:

  1. There is no such thing as required workflow. At least I cannot find one when I am configuring setting for a branch. What it does allow me is to setup a required check. It is very confusing and misleading, especially when there are more than one check (a.k.a. job) in a workflow (a.k.a. file).
  2. Ignoring any filters when a job is marked as required, means breaking a contract of the job configuration. It makes the "required check" feature confusing, introduces longer (than expected) check running times and may introduce extra costs.

Such decisions should be published for community feedback before they make intro the product.

@TWiStErRob
Copy link
Contributor

For 1), @minherz scroll up on the linked page for an explanation. Also notice the banner:

Note: Required workflows for GitHub Actions are in private beta and are not available for new signups.

@jsoref
Copy link
Contributor Author

jsoref commented Jul 17, 2023

@TWiStErRob I don't believe they existed when I opened this issue.

@minherz
Copy link

minherz commented Jul 19, 2023

@TWiStErRob you are right, it was my mistake. I am biased in that when I read public documentation it describes functionality that is available to public.

It is good to see that the original problem still exists in its original form. It is bad that there is no solution to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content This issue or pull request belongs to the Docs Content team help wanted Anyone is welcome to open a pull request to fix this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

11 participants