Skip to content

Conversation

trueNAHO
Copy link
Member

@trueNAHO trueNAHO commented Aug 18, 2025

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

Notify Maintainers

@awwpotato @danth

Exclude automated backports to avoid re-notifying original reviewers
on trivial backports.
@trueNAHO trueNAHO requested review from danth and awwpotato August 18, 2025 20:22
@trueNAHO trueNAHO added the backport: release-25.05 Changes to release-25.05 stable branch label Aug 18, 2025
@stylix-automation stylix-automation bot added the topic: ci /.github/ subsystem label Aug 18, 2025
@danth
Copy link
Collaborator

danth commented Aug 18, 2025

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.

@trueNAHO
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport: release-25.05 Changes to release-25.05 stable branch topic: ci /.github/ subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants