zhengruifeng commented on code in PR #58198:
URL: https://github.com/apache/spark/pull/58198#discussion_r3848994683


##########
dev/merge_spark_pr.py:
##########
@@ -548,12 +583,39 @@ def fail(msg):
     sys.exit(-1)
 
 
-def run_cmd(cmd):
-    print(cmd)
-    if isinstance(cmd, list):
-        return subprocess.check_output(cmd).decode("utf-8")
-    else:
-        return subprocess.check_output(cmd.split(" ")).decode("utf-8")
+class Git:
+    """Runs git for the merge script -- the single seam for all git I/O.

Review Comment:
   This `all git I/O` wording is broader than the current script: 
`check_script_up_to_date()` still shells out to `git merge-base` directly. I 
would narrow this to the merge-flow git commands so future changes do not 
assume this client covers every git invocation.
   
   ```suggestion
       """Runs merge-flow git commands for the merge script.
   ```



##########
dev/merge_spark_pr.py:
##########
@@ -1312,23 +1350,99 @@ def choose_jira_assignee(issue):
             print("Error assigning JIRA, try again (or leave blank and fix 
manually)")
 
 
-def grant_contributor_role(user: str):
-    role = asf_jira.project_role("SPARK", 10010)
-    role.add_user(user)
-    print("Successfully added user '%s' to contributors role" % user)
-
+class Jira:
+    """ASF JIRA writes used by the merge script -- the single seam for JIRA 
mutations.
 
-def assign_issue(issue: int, assignee: str) -> bool:
-    """
-    Assign an issue to a user, which is a shorthand for 
jira.client.JIRA.assign_issue.
-    The original one has an issue that it will search users again and only 
choose the assignee
-    from 20 candidates. If it's unmatched, it picks the head blindly. In our 
case, the assignee
-    is already resolved.
+    Reads stay on the module-level asf_jira client; only the writes 
(components, fix versions,

Review Comment:
   This reads broader than the implementation. `add_fix_versions()` and 
`resolve_issue()` both read the issue back through `self._client.issue(...)` 
after writing, so I would narrow this to the pre-write lookups or call out 
those readbacks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to