Skip to content

Commit 4b06855

Browse files
committed
ci/tag-maintainers: refactor managing reviewers
Move to separate script that looks at history of requests to determine who needs to be removed. We will not remove reviews from those who were manually requested.
1 parent c4353d0 commit 4b06855

File tree

2 files changed

+269
-72
lines changed

2 files changed

+269
-72
lines changed

.github/workflows/tag-maintainers.yml

Lines changed: 16 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -73,79 +73,23 @@ jobs:
7373
7474
echo "maintainers=$MAINTAINERS_LIST" >> "$GITHUB_OUTPUT"
7575
76-
# Get lists of existing reviewers to avoid duplicates.
77-
- name: Get current reviewers
78-
id: current-reviewers
79-
env:
80-
GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }}
81-
PR_NUM: ${{ github.event.pull_request.number }}
82-
REPO: ${{ github.repository }}
83-
run: |
84-
PENDING_REVIEWERS=$(gh pr view "$PR_NUM" --json reviewRequests --jq '.reviewRequests[].login')
85-
PAST_REVIEWERS=$(gh api "repos/$REPO/pulls/$PR_NUM/reviews" --jq '.[].user.login')
86-
USERS_TO_EXCLUDE=$(printf "%s\n%s" "$PENDING_REVIEWERS" "$PAST_REVIEWERS" | sort -u)
87-
88-
{
89-
echo "pending_reviewers<<EOF"
90-
echo "$PENDING_REVIEWERS"
91-
echo EOF
92-
echo "users_to_exclude<<EOF"
93-
echo "$USERS_TO_EXCLUDE"
94-
echo EOF
95-
} >> $GITHUB_OUTPUT
96-
97-
echo "Current pending reviewers: $PENDING_REVIEWERS"
98-
echo "Complete list of users to exclude: $USERS_TO_EXCLUDE"
99-
100-
# Filter the maintainer list to only include repository collaborators.
101-
# You can only request reviews from users with at least triage permissions.
102-
- name: Check maintainer collaborator status
103-
id: check-collaborators
76+
# Manage reviewers
77+
- name: Manage reviewers
10478
env:
10579
GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }}
80+
OWNER: ${{ github.repository_owner }}
81+
REPO: ${{ github.event.repository.name }}
82+
PR_NUMBER: ${{ github.event.pull_request.number }}
83+
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
10684
MAINTAINERS: ${{ steps.extract-maintainers.outputs.maintainers }}
107-
USERS_TO_EXCLUDE: ${{ steps.current-reviewers.outputs.users_to_exclude }}
108-
REPO: "${{ github.repository }}"
85+
CHANGED_FILES: ${{ steps.changed-files.outputs.changed_files }}
86+
BOT_NAME: ${{ steps.app-token.outputs.app-slug || 'github-actions' }}
10987
run: |
110-
NEW_REVIEWERS=()
111-
112-
# If there are no maintainers, exit early.
113-
if [[ -z "$MAINTAINERS" ]]; then
114-
echo "No maintainers to check."
115-
echo "new_reviewers=" >> "$GITHUB_OUTPUT"
116-
exit 0
117-
fi
118-
119-
for MAINTAINER in $MAINTAINERS; do
120-
if echo "$USERS_TO_EXCLUDE" | grep -q -w "$MAINTAINER"; then
121-
echo "$MAINTAINER is already involved in the review, skipping."
122-
continue
123-
fi
124-
125-
echo "Checking if $MAINTAINER is a collaborator..."
126-
if gh api "/repos/$REPO/collaborators/$MAINTAINER" --silent; then
127-
echo "User $MAINTAINER is a collaborator, adding to new reviewers list."
128-
NEW_REVIEWERS+=("$MAINTAINER")
129-
else
130-
echo "User $MAINTAINER is not a repository collaborator, skipping."
131-
fi
132-
done
133-
134-
NEW_REVIEWERS_LIST=$(printf "%s " "${NEW_REVIEWERS[@]}")
135-
echo "new_reviewers=${NEW_REVIEWERS_LIST% }" >> "$GITHUB_OUTPUT"
136-
echo "New reviewers to add: ${NEW_REVIEWERS_LIST% }"
137-
138-
# Add the new, filtered list of maintainers as reviewers to the PR.
139-
- name: Add new reviewers
140-
env:
141-
GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }}
142-
NEW_REVIEWERS: ${{ steps.check-collaborators.outputs.new_reviewers }}
143-
PR_NUM: ${{ github.event.pull_request.number }}
144-
run: |
145-
if [[ -n "$NEW_REVIEWERS" ]]; then
146-
REVIEWERS_CSV=$(echo "$NEW_REVIEWERS" | tr ' ' ',')
147-
echo "Requesting reviews from: $REVIEWERS_CSV"
148-
gh pr edit "$PR_NUM" --add-reviewer "$REVIEWERS_CSV"
149-
else
150-
echo "No new reviewers to add."
151-
fi
88+
./ci/tag-maintainers/manage-reviewers.py \
89+
--owner "$OWNER" \
90+
--repo "$REPO" \
91+
--pr-number "$PR_NUMBER" \
92+
--pr-author "$PR_AUTHOR" \
93+
--current-maintainers "$MAINTAINERS" \
94+
--changed-files "$CHANGED_FILES" \
95+
--bot-user-name "$BOT_NAME"
Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Manage pull request reviewers for Nixvim.
4+
5+
This script handles the reviewer management logic from the tag-maintainers workflow,
6+
including checking for manually requested reviewers and managing removals.
7+
"""
8+
9+
import argparse
10+
import json
11+
import logging
12+
import subprocess
13+
import sys
14+
from typing import Final
15+
16+
logging.basicConfig(
17+
level=logging.INFO,
18+
format="%(message)s",
19+
stream=sys.stderr,
20+
)
21+
22+
MANUAL_REVIEW_REQUEST_QUERY: Final[str] = """
23+
query($owner: String!, $repo: String!, $prNumber: Int!) {
24+
repository(owner: $owner, name: $repo) {
25+
pullRequest(number: $prNumber) {
26+
timelineItems(last: 250, itemTypes: [REVIEW_REQUESTED_EVENT]) {
27+
nodes {
28+
... on ReviewRequestedEvent {
29+
actor {
30+
__typename
31+
login
32+
}
33+
requestedReviewer {
34+
... on User { login }
35+
... on Bot { login }
36+
}
37+
}
38+
}
39+
}
40+
}
41+
}
42+
}
43+
"""
44+
45+
46+
class GHError(Exception):
47+
"""Custom exception for errors related to 'gh' CLI commands."""
48+
pass
49+
50+
51+
def run_gh_command(
52+
args: list[str],
53+
input_data: str | None = None,
54+
check: bool = True,
55+
) -> subprocess.CompletedProcess:
56+
"""Runs a GitHub CLI command and returns the CompletedProcess object."""
57+
command = ["gh"] + args
58+
try:
59+
result = subprocess.run(
60+
command,
61+
input=input_data,
62+
capture_output=True,
63+
text=True,
64+
check=check,
65+
)
66+
return result
67+
except subprocess.CalledProcessError as e:
68+
logging.error("Error running command: %s", " ".join(command))
69+
logging.error("Stderr: %s", e.stderr.strip())
70+
raise GHError(f"Failed to execute gh command: {e}") from e
71+
72+
73+
def get_manually_requested_reviewers(
74+
owner: str, repo: str, pr_number: int, bot_user_name: str
75+
) -> set[str]:
76+
"""Fetches a set of reviewers who were manually requested by someone other than the bot."""
77+
try:
78+
result = run_gh_command([
79+
"api", "graphql",
80+
"-f", f"query={MANUAL_REVIEW_REQUEST_QUERY}",
81+
"-F", f"owner={owner}",
82+
"-F", f"repo={repo}",
83+
"-F", f"prNumber={pr_number}",
84+
])
85+
data = json.loads(result.stdout)
86+
nodes = data.get("data", {}).get("repository", {}).get("pullRequest", {}).get("timelineItems", {}).get("nodes", [])
87+
88+
manually_requested = {
89+
node["requestedReviewer"]["login"]
90+
for node in nodes
91+
if node and node.get("requestedReviewer") and node.get("actor", {}).get("login") != bot_user_name
92+
}
93+
return manually_requested
94+
except (GHError, json.JSONDecodeError, KeyError) as e:
95+
logging.error("Could not determine manually requested reviewers: %s", e)
96+
return set()
97+
98+
99+
def get_users_from_gh(args: list[str], error_message: str) -> set[str]:
100+
"""A generic helper to get a set of users from a 'gh' command."""
101+
try:
102+
result = run_gh_command(args)
103+
return {user.strip() for user in result.stdout.split("\n") if user.strip()}
104+
except GHError as e:
105+
logging.error("%s: %s", error_message, e)
106+
return set()
107+
108+
109+
def get_pending_reviewers(pr_number: int) -> set[str]:
110+
"""Gets the set of currently pending reviewers for a PR."""
111+
return get_users_from_gh(
112+
["pr", "view", str(pr_number), "--json", "reviewRequests", "--jq", ".reviewRequests[].login"],
113+
"Error getting pending reviewers",
114+
)
115+
116+
117+
def get_past_reviewers(owner: str, repo: str, pr_number: int) -> set[str]:
118+
"""Gets the set of users who have already reviewed the PR."""
119+
return get_users_from_gh(
120+
["api", f"repos/{owner}/{repo}/pulls/{pr_number}/reviews", "--jq", ".[].user.login"],
121+
"Error getting past reviewers",
122+
)
123+
124+
125+
def is_collaborator(owner: str, repo: str, username: str) -> bool:
126+
"""
127+
Checks if a user is a collaborator on the repository.
128+
Handles 404 as a non-collaborator, while other errors are raised.
129+
"""
130+
result = run_gh_command(
131+
["api", f"repos/{owner}/{repo}/collaborators/{username}"],
132+
check=False
133+
)
134+
135+
if result.returncode == 0:
136+
return True
137+
138+
if "HTTP 404" in result.stderr:
139+
logging.error(
140+
"'%s' is not a collaborator in this repository.", username
141+
)
142+
return False
143+
else:
144+
logging.error(
145+
"Unexpected error checking collaborator status for '%s'.", username
146+
)
147+
logging.error("Stderr: %s", result.stderr.strip())
148+
raise GHError(
149+
f"Unexpected API error for user '{username}': {result.stderr.strip()}"
150+
)
151+
152+
153+
def update_reviewers(
154+
pr_number: int,
155+
reviewers_to_add: set[str] | None = None,
156+
reviewers_to_remove: set[str] | None = None,
157+
owner: str | None = None,
158+
repo: str | None = None,
159+
) -> None:
160+
"""Adds or removes reviewers from a PR in a single operation per action."""
161+
if reviewers_to_add:
162+
logging.info("Requesting reviews from: %s", ", ".join(reviewers_to_add))
163+
try:
164+
run_gh_command([
165+
"pr", "edit", str(pr_number),
166+
"--add-reviewer", ",".join(reviewers_to_add)
167+
])
168+
except GHError as e:
169+
logging.error("Failed to add reviewers: %s", e)
170+
171+
if reviewers_to_remove and owner and repo:
172+
logging.info("Removing review requests from: %s", ", ".join(reviewers_to_remove))
173+
payload = json.dumps({"reviewers": list(reviewers_to_remove)})
174+
try:
175+
run_gh_command(
176+
[
177+
"api", "--method", "DELETE",
178+
f"repos/{owner}/{repo}/pulls/{pr_number}/requested_reviewers",
179+
"--input", "-",
180+
],
181+
input_data=payload,
182+
)
183+
except GHError as e:
184+
logging.error("Failed to remove reviewers: %s", e)
185+
186+
187+
def main() -> None:
188+
"""Main function to handle command-line arguments and manage reviewers."""
189+
parser = argparse.ArgumentParser(description="Manage pull request reviewers for Nixvim.")
190+
parser.add_argument("--owner", required=True, help="Repository owner.")
191+
parser.add_argument("--repo", required=True, help="Repository name.")
192+
parser.add_argument("--pr-number", type=int, required=True, help="Pull request number.")
193+
parser.add_argument("--pr-author", required=True, help="PR author's username.")
194+
parser.add_argument("--current-maintainers", default="", help="Space-separated list of current maintainers.")
195+
parser.add_argument("--changed-files", default="", help="Newline-separated list of changed files.")
196+
parser.add_argument("--bot-user-name", default="", help="Bot user name to distinguish manual vs automated review requests.")
197+
args = parser.parse_args()
198+
199+
no_plugin_files = not args.changed_files.strip()
200+
201+
# --- 1. Fetch current state from GitHub ---
202+
maintainers: set[str] = set(args.current_maintainers.split())
203+
pending_reviewers = get_pending_reviewers(args.pr_number)
204+
past_reviewers = get_past_reviewers(args.owner, args.repo, args.pr_number)
205+
manually_requested = get_manually_requested_reviewers(args.owner, args.repo, args.pr_number, args.bot_user_name)
206+
207+
logging.info("Current Maintainers: %s", ' '.join(maintainers) or "None")
208+
logging.info("Pending Reviewers: %s", ' '.join(pending_reviewers) or "None")
209+
logging.info("Past Reviewers: %s", ' '.join(past_reviewers) or "None")
210+
logging.info("Manually Requested: %s", ' '.join(manually_requested) or "None")
211+
212+
# --- 2. Determine reviewers to remove ---
213+
reviewers_to_remove: set[str] = set()
214+
if no_plugin_files:
215+
reviewers_to_remove = pending_reviewers - manually_requested
216+
logging.info("No plugin files changed. Removing bot-requested reviewers.")
217+
else:
218+
outdated_reviewers = pending_reviewers - maintainers
219+
reviewers_to_remove = outdated_reviewers - manually_requested
220+
logging.info("Removing outdated bot-requested reviewers.")
221+
222+
if reviewers_to_remove:
223+
update_reviewers(
224+
args.pr_number,
225+
owner=args.owner,
226+
repo=args.repo,
227+
reviewers_to_remove=reviewers_to_remove
228+
)
229+
else:
230+
logging.info("No reviewers to remove.")
231+
232+
# --- 3. Determine new reviewers to add ---
233+
reviewers_to_add: set[str] = set()
234+
if not no_plugin_files and maintainers:
235+
users_to_exclude = {args.pr_author} | past_reviewers | pending_reviewers
236+
potential_reviewers = maintainers - users_to_exclude
237+
238+
reviewers_to_add = {
239+
user for user in potential_reviewers if is_collaborator(args.owner, args.repo, user)
240+
}
241+
242+
non_collaborators = potential_reviewers - reviewers_to_add
243+
if non_collaborators:
244+
logging.warning("Ignoring non-collaborators: %s", ", ".join(non_collaborators))
245+
246+
if reviewers_to_add:
247+
update_reviewers(args.pr_number, reviewers_to_add=reviewers_to_add)
248+
else:
249+
logging.info("No new reviewers to add.")
250+
251+
252+
if __name__ == "__main__":
253+
main()

0 commit comments

Comments
 (0)