upload: Don't re-add assignees/reviewers that were removed

If an assignee/reviewer was removed intentionally, don't re-add them.
Add a --force-reviewers flag to bypass this that defaults to false

Author: jerry-skydio

docs/upload.md

@@ -9,7 +9,7 @@ revup upload - Modify or create code reviews.
 `[--rebase] [--relative-chain] [--skip-confirm] [--dry-run] [--push-only]`
 `[--status] [--no-update-pr-body] [--review-graph]`
 `[--trim-tags] [--create-local-branches] [--patchsets] [--auto-add-users=<o>]`
-`[--labels=<labels>] [<topics>]`
+`[--force-reviewers] [--labels=<labels>] [<topics>]`
 
 # DESCRIPTION
 
@@ -209,6 +209,9 @@ the above section for details.
 : Specifies a comma separated list of colon separated username mappings. These
 mappings are used to transform usernames specified in Reviewers/Assignees.
 
+**--force-reviewers**
+: Re-add reviewers and assignees even if they were manually removed from the PR.
+
 **--auto-add-users**
 : If "no", do nothing extra. If "r2a", add users from the Reviewers tag as assignees.
 If "a2r", add users from the Assignees tag as reviewers. If "both", do both of the previous.

revup/github_utils.py

@@ -50,6 +50,10 @@ class PrInfo:
     assignee_ids: Set[str] = field(default_factory=set)
     labels: Set[str] = field(default_factory=set)
     label_ids: Set[str] = field(default_factory=set)
+    removed_reviewers: Set[str] = field(default_factory=set)
+    removed_reviewer_ids: Set[str] = field(default_factory=set)
+    removed_assignees: Set[str] = field(default_factory=set)
+    removed_assignee_ids: Set[str] = field(default_factory=set)
     is_draft: bool = False
     comments: List[PrComment] = field(default_factory=list)
 
@@ -225,6 +229,26 @@ async def query_everything(
                         }}
                     }}
                 }}
+                timelineItems(itemTypes: [REVIEW_REQUEST_REMOVED_EVENT, UNASSIGNED_EVENT], first: 50) {{
+                    nodes {{
+                        ... on ReviewRequestRemovedEvent {{
+                            requestedReviewer {{
+                                ... on User {{
+                                    login
+                                    id
+                                }}
+                            }}
+                        }}
+                        ... on UnassignedEvent {{
+                            assignee {{
+                                ... on User {{
+                                    login
+                                    id
+                                }}
+                            }}
+                        }}
+                    }}
+                }}
                 latestReviews (first: 25) {{
                     nodes {{
                         author {{
@@ -317,6 +341,20 @@ async def query_everything(
             for c in this_node["comments"]["nodes"]:
                 comments.append(PrComment(c["body"], c["id"]))
 
+            removed_reviewers: Set[str] = set()
+            removed_reviewer_ids: Set[str] = set()
+            removed_assignees: Set[str] = set()
+            removed_assignee_ids: Set[str] = set()
+            for event in this_node["timelineItems"]["nodes"]:
+                rr = event.get("requestedReviewer")
+                if rr and "login" in rr and rr["login"] not in reviewers:
+                    removed_reviewers.add(rr["login"])
+                    removed_reviewer_ids.add(rr["id"])
+                assignee = event.get("assignee")
+                if assignee and "login" in assignee and assignee["login"] not in assignees:
+                    removed_assignees.add(assignee["login"])
+                    removed_assignee_ids.add(assignee["id"])
+
             prs.append(
                 PrInfo(
                     id=this_node["id"],
@@ -333,6 +371,10 @@ async def query_everything(
                     assignee_ids=assignee_ids,
                     labels=pr_labels,
                     label_ids=pr_label_ids,
+                    removed_reviewers=removed_reviewers,
+                    removed_reviewer_ids=removed_reviewer_ids,
+                    removed_assignees=removed_assignees,
+                    removed_assignee_ids=removed_assignee_ids,
                     is_draft=this_node["isDraft"],
                     state=this_node["state"],
                     comments=comments,

revup/revup.py

@@ -192,6 +192,7 @@ def build_parser() -> Tuple[RevupArgParser, List[RevupArgParser]]:
     upload_parser.add_argument("--pre-upload", "-p")
     upload_parser.add_argument("--relative-chain", "-c", action="store_true")
     upload_parser.add_argument("--auto-topic", "-a", action="store_true")
+    upload_parser.add_argument("--force-reviewers", action="store_true")
     upload_parser.add_argument("--head", default="HEAD")
 
     restack_parser.add_argument("--topicless-last", "-t", action="store_true")

revup/topic_stack.py

@@ -1142,6 +1142,7 @@ async def query_github(self) -> None:
     def populate_update_info(
         self,
         update_pr_body_arg: bool,
+        force_reviewers: bool = False,
     ) -> None:
         """
         Populate information necessary to do PR creation / update in github.
@@ -1214,6 +1215,19 @@ def populate_update_info(
                     topic.tags[TAG_ASSIGNEE], self.names_to_logins
                 ).difference(review.pr_info.assignees)
 
+                if not force_reviewers:
+                    removed = reviewer_logins & review.pr_info.removed_reviewers
+                    if removed:
+                        logging.warning(f"Skipping removed reviewer(s) {removed} for {topic.name}")
+                        reviewer_logins -= removed
+                        reviewer_ids -= review.pr_info.removed_reviewer_ids
+
+                    removed = assignee_logins & review.pr_info.removed_assignees
+                    if removed:
+                        logging.warning(f"Skipping removed assignee(s) {removed} for {topic.name}")
+                        assignee_logins -= removed
+                        assignee_ids -= review.pr_info.removed_assignee_ids
+
                 if TAG_UPDATE_PR_BODY in topic.tags:
                     update_pr_body = min(topic.tags[TAG_UPDATE_PR_BODY]).lower() == "true"
                 else:

revup/upload.py

@@ -70,7 +70,7 @@ async def main(
         return 0
 
     if not args.push_only:
-        topics.populate_update_info(args.update_pr_body)
+        topics.populate_update_info(args.update_pr_body, args.force_reviewers)
     if not args.skip_confirm and topics.num_reviews_changed() > 0:
         topics.print(not args.verbose)
         if git_ctx.sh.wait_for_confirmation():