The failing check-cherry-picks job is expected, it shows how the output would look like, when somebody did not cherry-pick correctly. Maybe we want to adjust the message slightly to hint at how to cherry-pick correctly, instead?

After a bit of back and forth the commit to speed up the cherry-pick workflow should now work correctly as well.

Added two more commits to (1) allow cherry-picking from haskell-updates and python-updates and (2) to make the script much faster for the case when the commit is not found on master or staging immediately.

Successfully created backport PR for release-25.05:

• [Backport release-25.05] ci/check-cherry-picks: fail without proper cherry-pick #411737

Thanks for your work on this <3

the last one introduces a change of behavior: [...]

If I remember correctly when we initially introduced it we were afraid it was going to generate too much noise and wanted to first focus on stuff announcing themselves as cherry picks but changing all the content. I guess we can try a more aggressive approach and see what happen :)

Maybe we want to adjust the message slightly to hint at how to cherry-pick correctly, instead?

Referencing https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#how-to-backport-pull-requests would be nice indeed.

How hard is it to make this CI check add a comment, like the "hey you didn't get the merge target of the PR right" check does? That would be a wonderful place to both link to the appropriate documentation and include a checkbox that a committer could check showing they paid attention to the matter.

How hard is it to make this CI check add a comment, like the "hey you didn't get the merge target of the PR right" check does?

We can't be 100% certain that this isn't a proper commit to the stable branches, which can't be cherry-picked. Although, I'd say those are rather infrequent. The most likely case, by far, is that contributors don't know they should have cherry-picked.

So yeah, I think we could do that.

include a checkbox that a committer could check showing they paid attention to the matter.

I don't think we'd need that, because the CI job would still fail (?). Once this is fixed, the job would pass... so CI paid attention to that matter!

I don't think we'd need that, because the CI job would still fail (?). Once this is fixed, the job would pass... so CI paid attention to that matter!

Yeah, I was imagining something like the checkbox that's already on the automated backports: "I certify that this is kosher" which mostly gets ignored but can be checked by the savvy or rule-followers.

It's pretty low value. What I'm trying to get at is something that others can see that indicates the difference between inattention to detail (the CI failed but was ignored) and intentionality (the CI failed, and I'm telling everyone there's a reason that's OK.) Today only a few (like yourself!) comment to let the reviewer know what's up. My appreciation of your care is what prompts the suggestion.

It's pretty low value. What I'm trying to get at is something that others can see that indicates the difference between inattention to detail (the CI failed but was ignored) and intentionality (the CI failed, and I'm telling everyone there's a reason that's OK.)

I see. I think you mean to post the comment whenever the cherry-pick job fails, while I'd only post a comment if it fails for the reason "couldn't find a commit hash in commit message" (aka, did you cherry-pick with -x?). The most frequent (and legitimate) failure of this job is when the commit was properly cherry-picked, but differs in content (aka merge conflicts had to be resolved). The question is whether we'd want to post a comment in that case, too?

Since the current error message says "Note this should not necessarily be treated as a hard fail, but a reviewer's attention should be drawn to it and github actions have no way of doing that but to raise a 'failure'"... maybe we actually want a comment instead of a failure?

So if we don't fail the check anymore, but instead show a single comment (which would be updated on subsequent runs) with all the details of the check... that would make it much easier to check those differences are valid. We wouldn't have to go through the job logs anymore...

And yes, in this case I can see the checkbox being helpful as a sign of "yeah, I checked this".

I'm just wondering whether a committer can check a box in a comment which is not their own...

• Could anybody please try to check this box? :)

maybe we actually want a comment instead of a failure?

I can see some obvious benefits to that approach. There are so many fewer clicks to read the PR body page (it's where the merge button is!) that information surfaced there is higher value by that fact alone. Add on the ability to link, have informative text, and be interactive and the value rises even higher.

The failure case, in my mind, ought to be "DO NOT MERGE THIS PR" if it's surfaced in that way. Such cases might exist, but none are coming to mind.

Also considering that people grow a habit out of merging a failed CI check (it's the cherry-pick job, I checked it, all good). Next time, maybe two jobs fail, but they didn't realize. Or maybe it's a different job. People should ideally only merge all-green PRs, makes them more aware of when things are actually broken.

I briefly wondered whether the GitHub App, which posts the comment, could "Request Changes" on the PR with the comment. Not sure whether that would work and whether GitHub Actions would consider it "blocking". If that was the case, the comment could have a little howto as "if you're ok with this diff, you can discard the request for changes by clicking here and there". This would essentially become a mandatory checkbox!

I'll play around with that a bit :D

"if you're ok with this diff, you can discard the request for changes by clicking here and there". This would essentially become a mandatory checkbox!

That's a really nifty idea. I like it a lot! We might have to give the bot more power than it currently has, but I support it.

I agree that a failing check feels wrong here and a comment would be better.

This could also block enabling "require status checks to pass".

Tangent:

In rulesets you can choose which status checks are required, however IIRC that's not the case with old school branch protection.

If we ever wanted to enable GitHub merge queues, I believe that'd currently require using branch protection instead of rulesets.

I took a shot at this in #412068. The part that requests reviews was simple, but much harder to get sensible markdown out of that cherry-pick script. I like the result, though :)