CI: pass with comment on linkcheck fail

[sam-maloney]

Problem: linkcheck fails frequently for various out-of-our-control reasons

Make the workflow pass regardless of make linkcheck outcome, but post a comment to the PR notifying if linkcheck failed.

Such a workaround is necessary because unfortunately GitHub CI does not natively support a sensible allow-failure type option, see e.g. actions/runner#2347 or https://github.com/orgs/community/discussions/15452

[Update]
It also turns out that workflows on PRs triggered from external forks do not have permission to create comments, so the attempt in the first commit is insufficient.. Instead a 2nd workflow is required, which is triggered by the completion of the main workflow, but since it is triggered by the target repo itself, this 2nd workflow should have permission to comment 🤞
This is based on: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/

[sam-maloney]

I've tested that this works on my own forked repo, but of course until the second workflow file is merged here I can only hope that it actually solves the permissions as intended.

[grondo]

Another simpler approach would be to drop the linkcheck requirement from .mergify.yml and just treat the error as a warning. Though I do like the idea of a PR comment much better, perhaps the 2nd workflow is overkill in this situation.

I'm fine either way, though. Perhaps others can chime in on their preferences.

In any event, dropping the explicit required checks in .mergify.yml in preference for branch protections like we've done in other projects is probably warranted here, so I can make a PR for that.

[sam-maloney]

I completely agree that it's overkill, haha! But it was the only way I could find to have the workflow show as 'passing' even if the linkcheck failed while still actually providing a warning that it had failed, and by that point I was in too deep and figured I'd just try implementing it to learn more about GitHub's CI 😁 Feel free to just make a simpler PR and close this though, if you prefer!

Other options I saw were just to use make linkcheck || true to always pass, but then you have to manually check the logs to even see the status; or as you suggested, make the linkcheck optional for mergify so it doesn't actually stop merging, but then the workflow will be frequently displayed as failing (which isn't the end of the world, but does seem to defeat the purpose of the status marks if every merged PR has a red X next to it!)

[grondo]

make the linkcheck optional for mergify so it doesn't actually stop merging, but then the workflow will be frequently displayed as failing (which isn't the end of the world, but does seem to defeat the purpose of the status marks if every merged PR has a red X next to it!)

This change has been enabled with the merge of #464, but it is good practice to move the status checks to branch protections anyway since it is easier to keep up to date. Your point about the status checks almost always being red is salient. I think we should accept this PR and see how it goes.

Thanks!

[grondo]

I'm going to rebase then merge this @sam-maloney. Hope that's ok.

[grondo]

Oh, new validator typos support caught one typo in a commit message.

[sam-maloney]

I'm going to rebase then merge this @sam-maloney. Hope that's ok.

Always OK with me to rebase/edit/mangle my PRs in any way you see fit, haha! 😁