Allow skipping notability checks using label.

[reitermarkus]

Needed for cases like https://github.com/Homebrew/homebrew-cask/pull/130308.

[SMillerDev]

Do we really need a label though? As a maintainer we can just choose to merge regardless.

[reitermarkus]

I am of the opinion that no PRs should be failing CI before merging, so applying a label instead of merging a failing PR is preferable.

[SMillerDev]

Might be better to tweak the warning level then. Since a label to disable a test isn't really fixing the issue. It's just hiding it.

[Bo98]

This would be beneficial to have so that we can re-enable requiring green CI here. I know things have been accidentally merged because they appear green during a GitHub Actions outage. This happens less often in Homebrew/core because our automerge there requires bottle artifacts, which can only be present if CI was run.

Being able to override audit failures was one of the major blockers that required that enforcement to be disabled, and I was shown the notability audit as an example of that. I'm not a cask maintainer though so don't know the fine details of what comes up in PRs.

[reitermarkus]

Might be better to tweak the warning level then. Since a label to disable a test isn't really fixing the issue.

I'm not sure what changing the warning level would do in this case?

The specific problem in the case of https://github.com/Homebrew/homebrew-cask/pull/130308 is that the repository used for hosting the downloads differs from the code for the source code of the application, which has vastly more stars and forks.

It's just hiding it.

The label should of course only be used in those cases which absolutely require it. And in doing so, it's doing the opposite of hiding it, it's showing the manual review of a maintainer as a visible label.

[reitermarkus]

Being able to override audit failures was one of the major blockers that required that enforcement to be disabled

Yeah, I don't agree with the decision to disable required CI checks. In my opinion, if CI is failing, it either needs to be fixed or an exception added.

[bevanjkay]

I am of the opinion that no PRs should be failing CI before merging, so applying a label instead of merging a failing PR is preferable.

I agree with this sentiment. It makes for a more verbose approach, as it is an express statement to say the maintainer is aware of the audit failure but intends for the PR to be merged regardless. Where just clicking squash and merge on a failing PR shows that potentially the maintainer was aware, but also may have just not noticed...

[carlocab]

I am of the opinion that no PRs should be failing CI before merging, so applying a label instead of merging a failing PR is preferable.

Yes, agreed. I think turning off the green CI requirement also

1. interacts poorly with GitHub auto-merge (since enabling it and then approving will not wait for CI to go green)
2. allows you to merge PRs without ever even looking at them (when, previously, the repo PR requirements required you to at least look at the "Files" tab to approve)

Some of the above is probably from a relaxation of PR merge requirements, but that seems to be bound up in allowing us to merge failing CI.

[Bo98]

2. allows you to merge PRs without ever even looking at them (when, previously, the repo PR requirements required you to at least look at the "Files" tab to approve)

Approval is still a requirement for merging, unless you are counting brew pr-publish.

Yeah, I don't agree with the decision to disable required CI checks.

To be clear: it was never meant to be a permanent change. It was always meant to be temporary (it was something actively blocking several PRs) until something like this was implemented, but there was unfortunately no follow up interest.

[carlocab]

Approval is still a requirement for merging, unless you are counting brew pr-publish.

Has that setting been applied to other homebrew-cask-* repos? The "Squash and merge" button is green for me here, and there are no reviews: https://github.com/Homebrew/homebrew-cask-fonts/pull/6223

[Bo98]

It seems not. Not sure why. I've applied it to all cask repos now - let me know if something breaks.

[MikeMcQuaid]

I like this idea, nice work @reitermarkus. Would be good to adopt for formulae, too.

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[MikeMcQuaid]

Let's close this out and reopen it when it's ready for merge.