git-workflow/autogits
SHA256
21
2
Fork 16
Code Issues 44 Pull Requests 19 Actions Projects 2 Activity

WIP: pr: enforce all maintainer approvals when ReviewRequired is true #154

Closed
andriinikitin wants to merge 1 commits from pr-review-required-tweak into main
pull from: pr-review-required-tweak
merge into: git-workflow:main
Conversation 1 Commits 1 Files Changed 4 +62 -11
andriinikitin commented 2026-03-03 15:58:06 +01:00
Member
Copy Link
  • Update MaintainershipMap.IsApproved to require approvals from all listed
    maintainers (excluding the submitter) when ReviewRequired is enabled.
  • Split ReviewInterface logic to differentiate between approvals and
    rejections by adding IsRejectedBy and restricting IsReviewedBy to
    the APPROVED state.
  • Fix premature removal of maintainer review requests in
    FindMissingAndExtraReviewers; ensure requests remain active if
    ReviewRequired is true and approvals are still missing.
  • Ensure any maintainer rejection correctly triggers the cleanup of
    other redundant review requests.

Analysis Summary
The failures in test_006 and test_007 were caused by two primary logic bugs:

  1. Conflated Review States: IsReviewedBy treated both "Approved" and "Request Changes" as the same state. This meant a rejection didn't properly trigger the "cleanup" logic, and an approval was indistinguishable from a rejection in several checks.
  2. Greedy Reviewer Removal: The bot assumed that any maintainer approval meant all other maintainer requests could be removed. This contradicted the ReviewRequired flag, which (as seen in test_007) is intended to mandate multiple/all maintainer approvals before the PR is considered ready.
- Update MaintainershipMap.IsApproved to require approvals from all listed maintainers (excluding the submitter) when ReviewRequired is enabled. - Split ReviewInterface logic to differentiate between approvals and rejections by adding IsRejectedBy and restricting IsReviewedBy to the APPROVED state. - Fix premature removal of maintainer review requests in FindMissingAndExtraReviewers; ensure requests remain active if ReviewRequired is true and approvals are still missing. - Ensure any maintainer rejection correctly triggers the cleanup of other redundant review requests. Analysis Summary The failures in test_006 and test_007 were caused by two primary logic bugs: 1. Conflated Review States: IsReviewedBy treated both "Approved" and "Request Changes" as the same state. This meant a rejection didn't properly trigger the "cleanup" logic, and an approval was indistinguishable from a rejection in several checks. 2. Greedy Reviewer Removal: The bot assumed that any maintainer approval meant all other maintainer requests could be removed. This contradicted the ReviewRequired flag, which (as seen in test_007) is intended to mandate multiple/all maintainer approvals before the PR is considered ready.
andriinikitin added 1 commit 2026-03-03 15:58:06 +01:00
pr: enforce all maintainer approvals when ReviewRequired is true
Some checks failed
go-generate-check / go-generate-check (pull_request) Successful in 8s
Details
Integration tests / t (pull_request) Has been cancelled
Details
a335d04031 
- Update MaintainershipMap.IsApproved to require approvals from all listed
  maintainers (excluding the submitter) when ReviewRequired is enabled.
- Split ReviewInterface logic to differentiate between approvals and
  rejections by adding IsRejectedBy and restricting IsReviewedBy to
  the APPROVED state.
- Fix premature removal of maintainer review requests in
  FindMissingAndExtraReviewers; ensure requests remain active if
  ReviewRequired is true and approvals are still missing.
- Ensure any maintainer rejection correctly triggers the cleanup of
  other redundant review requests.

Analysis Summary
The failures in test_006 and test_007 were caused by two primary logic bugs:
1. Conflated Review States: IsReviewedBy treated both "Approved" and "Request Changes" as the same state. This meant a rejection didn't properly trigger the "cleanup" logic, and an approval was indistinguishable from a rejection in several checks.
2. Greedy Reviewer Removal: The bot assumed that any maintainer approval meant all other maintainer requests could be removed. This contradicted the ReviewRequired flag, which (as seen in test_007) is intended to mandate multiple/all maintainer approvals before the PR is considered ready.
andriinikitin changed title from pr: enforce all maintainer approvals when ReviewRequired is true to WIP: pr: enforce all maintainer approvals when ReviewRequired is true 2026-03-03 15:58:10 +01:00
andriinikitin commented 2026-03-03 16:06:29 +01:00
Author
Member
Copy Link

closing for now, it doesn't look that the problem is fixed.

closing for now, it doesn't look that the problem is fixed.
andriinikitin closed this pull request 2026-03-03 16:06:29 +01:00
Some checks failed
go-generate-check / go-generate-check (pull_request) Successful in 8s
Details
Integration tests / t (pull_request) Has been cancelled
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
M0ses adamm adamm_super adrianSuSE andriinikitin atartamo bigironman dancermak darix dirkmueller dmach eroca jdsn jzerebecki mstrigl oertel smithfarm
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: git-workflow/autogits#154
Delete Branch "pr-review-required-tweak"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?