danth
ci: request-reviewers: exclude automated backports
Exclude automated backports to avoid re-notifying original reviewers
on trivial backports.
This implementation is untested, fragile, and subject to false positives. The current implementation is to start a conversation about whether this functionality is desirable.
The current !startsWith(github.event.pull_request.head.ref, 'backport-') guard is fragile because it does not check for the more restrictive backport-<HASH>-to-release-25.05 pattern. It is subject to false positives because manual backports might desirably follow the backport-<HASH>-to-release-25.05 naming convention, and manual backports may arbitrarily diverge from their original commits.
From a quick search, GitHub does not seem to provide a function to trivially match on the backport-<HASH>-to-release-25.05 pattern, although the following might do the job:
! (
startsWith(github.event.pull_request.head.ref, 'backport-') &&
endsWith(github.event.pull_request.head.ref, '-to-release-25.05')
)
Should manual backports request review by default?
The fragile backport-<HASH>-to-release-25.05 pattern matching could be replaced with a more sophisticated implementation where the "Backport" Action shares state with the "Request Reviewers" Action, possibly by having "Backport" add a custom dedicated label to its PRs, which "Request Reviewers" uses as guard.
Submission Checklist
- [X] I certify that I have the right to submit this contribution under the MIT license
- [X] Commit messages adhere to Stylix commit conventions
- [ ] Theming changes adhere to the Stylix style guide
- [ ] Changes have been tested locally
- [ ] Changes have been tested in testbeds
- [X] Each commit in this PR is suitable for backport to the current stable branch
Notify Maintainers
@awwpotato @danth
Should manual backports request review by default?
For manual backports, I think it's desirable to have another review, since it's possible to introduce bugs while resolving a merge conflict.
There's also the potential that things work differently on the release branch compared to master, which the module maintainer may be aware of and be able to point out before merging - this applies even to automated backports.
Should manual backports request review by default?
[...]
There's also the potential that things work differently on the release branch compared to
master, which the module maintainer may be aware of and be able to point out before merging - this applies even to automated backports.
True. However, IIRC automated backports have only very rarely (less than 5 times) had non-obvious problems that were somehow not caught by trivial CI failures. Seems like the question boils down to how much we want to bother module maintainers with a secondary potentially trivial review.
Should manual backports request review by default?
[...]
There's also the potential that things work differently on the release branch compared to
master, which the module maintainer may be aware of and be able to point out before merging - this applies even to automated backports.True. However, IIRC automated backports have only very rarely (less than 5 times) had non-obvious problems that were somehow not caught by trivial CI failures. Seems like the question boils down to how much we want to bother module maintainers with a secondary potentially trivial review.
Considering the current situation seems fine, I will close this proposal.