Skip to content

Conversation

@makubacki
Copy link
Member

Description

The add_reviewers_to_pr() function in GitHub.py did not compare all usernames without case sensitivity which could cause a reviewer that has already reviewed a pull request to be re-requested.

The occurred under the following conditions:

  • GetMaintainer.py returns usernames from Maintainers.txt (e.g. "user")
  • GitHub API returns usernames in their actual case (e.g. "User")
  • The exclusion filter used case-sensitive comparison so the match is not detected

Fixed by converting the exclusion set to lowercase and performing case-insensitive comparison when filtering for new reviewers.

  • Breaking change?
  • Impacts security?
  • Includes tests?

How This Was Tested

Integration Instructions

  • N/A

The `add_reviewers_to_pr()` function in GitHub.py did not compare
all usernames without case sensitivity which could cause a reviewer
that has already reviewed a pull request to be re-requested.

The occurred under the following conditions:

- GetMaintainer.py returns usernames from Maintainers.txt
  (e.g. "user")
- GitHub API returns usernames in their actual case (e.g. "User")
- The exclusion filter used case-sensitive comparison so the match
  is not detected

Fixed by converting the exclusion set to lowercase and performing
case-insensitive comparison when filtering for new reviewers.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
@makubacki makubacki added the push Auto push patch series in PR if all checks pass label Dec 1, 2025
@mergify
Copy link

mergify bot commented Dec 1, 2025

Merge Queue Status Beta

✅ The pull request has been merged

This pull request spent 3 hours 21 minutes 51 seconds in the queue, including 2 hours 8 minutes 36 seconds waiting for CI.
The checks were run in-place.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = tianocore.PatchCheck
    • check-neutral = tianocore.PatchCheck
    • check-skipped = tianocore.PatchCheck
  • any of [🛡 GitHub branch protection]:
    • check-success = ArmVirtPkg - Ubuntu GCC - PR
    • check-neutral = ArmVirtPkg - Ubuntu GCC - PR
    • check-skipped = ArmVirtPkg - Ubuntu GCC - PR
  • any of [🛡 GitHub branch protection]:
    • check-success = EmulatorPkg - Ubuntu GCC - PR
    • check-neutral = EmulatorPkg - Ubuntu GCC - PR
    • check-skipped = EmulatorPkg - Ubuntu GCC - PR
  • any of [🛡 GitHub branch protection]:
    • check-success = EmulatorPkg - Windows VS - PR
    • check-neutral = EmulatorPkg - Windows VS - PR
    • check-skipped = EmulatorPkg - Windows VS - PR
  • any of [🛡 GitHub branch protection]:
    • check-success = OvmfPkg - Ubuntu GCC - PR
    • check-neutral = OvmfPkg - Ubuntu GCC - PR
    • check-skipped = OvmfPkg - Ubuntu GCC - PR
  • any of [🛡 GitHub branch protection]:
    • check-success = OvmfPkg - Windows VS - PR
    • check-neutral = OvmfPkg - Windows VS - PR
    • check-skipped = OvmfPkg - Windows VS - PR
  • any of [🛡 GitHub branch protection]:
    • check-success = Windows VS - PR
    • check-neutral = Windows VS - PR
    • check-skipped = Windows VS - PR
  • any of [🛡 GitHub branch protection]:
    • check-success = Ubuntu GCC - PR
    • check-neutral = Ubuntu GCC - PR
    • check-skipped = Ubuntu GCC - PR
  • any of [🛡 GitHub branch protection]:
    • check-success = Validate Pull Request Formatting
    • check-neutral = Validate Pull Request Formatting
    • check-skipped = Validate Pull Request Formatting

@mergify mergify bot added the queued label Dec 1, 2025
@mergify mergify bot merged commit 43e86ce into tianocore:master Dec 1, 2025
117 checks passed
@mergify mergify bot removed the queued label Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

push Auto push patch series in PR if all checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants