Skip to content

Commit 52294a3

Browse files
committed
Remove gh_get_prs() duplicated logic
Pull Request: #20 (main)
1 parent 8b7007a commit 52294a3

File tree

1 file changed

+26
-111
lines changed

1 file changed

+26
-111
lines changed

git-grok

Lines changed: 26 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class Pr:
7979
url: str
8080
review_decision: PrReviewDecision
8181
state: Literal["OPEN", "CLOSED", "MERGED"]
82-
auto_merge_status: Literal["ENABLED", "DISABLED", "UNKNOWN"]
82+
auto_merge_status: Literal["ENABLED", "DISABLED"]
8383
labels: list[str]
8484

8585

@@ -274,7 +274,6 @@ class Main:
274274
return
275275

276276
task_upsert_labels = Task(self.gh_upsert_labels)
277-
task_validate_commits = Task(self.process_validate_commits, commits=commits)
278277
tasks_gh_get_pr = (
279278
dict(
280279
[
@@ -285,13 +284,13 @@ class Main:
285284
if not self.debug_force_push_branches
286285
else {}
287286
)
288-
289-
task_validate_commits.wait()
290287
task_upsert_labels.wait()
291288
prs_by_url = dict((url, task.wait()) for (url, task) in tasks_gh_get_pr.items())
292289

293290
self.debug_log_commits(commits=commits, prs_by_url=prs_by_url)
294291

292+
self.process_validate_commits(commits=commits, prs_by_url=prs_by_url)
293+
295294
# Inject "Pull Request" URL to commit descriptions starting from the
296295
# commit which doesn't have it. This is a heavy-weighted process which
297296
# is run for each commit in an interactive rebase, starting from bottom
@@ -338,12 +337,20 @@ class Main:
338337
#
339338
# Checks that the tool can process this stack of commits at all.
340339
#
341-
def process_validate_commits(self, *, commits: list[Commit]):
340+
def process_validate_commits(
341+
self,
342+
*,
343+
commits: list[Commit],
344+
prs_by_url: dict[str, Pr],
345+
):
342346
commits_chronological = list(reversed(commits))
343-
prs_chronological = self.gh_get_prs(commits=commits_chronological)
344-
for i, pr in list(enumerate(prs_chronological))[1:]:
347+
for i, commit in list(enumerate(commits_chronological))[1:]:
348+
pr = prs_by_url.get(commit.url, None) if commit.url else None
345349
if pr and pr.auto_merge_status == "ENABLED":
346-
prev_pr = prs_chronological[i - 1]
350+
prev_commit = commits_chronological[i - 1]
351+
prev_pr = (
352+
prs_by_url.get(prev_commit.url, None) if prev_commit.url else None
353+
)
347354
if not prev_pr or prev_pr.head_branch != pr.base_branch:
348355
raise UserException(
349356
f"PR {pr.url} is auto-mergeable.\n"
@@ -701,6 +708,15 @@ class Main:
701708
#
702709
# Reads PR info from GitHub.
703710
#
711+
# We read PRs one by one and not in GraphQL-bulk for 3 reasons:
712+
# - Granular caching. We use cache_through() for each PR, so in the
713+
# consequent interactive rebase call, it will be reused (and also, it will
714+
# be reused in other places where we need to read a PR info).
715+
# - It is not much worse in performance, because we run the initial
716+
# gh_get_pr() in parallel, in Task threads.
717+
# - Simplicity reason: we want to avoid GraphQL code duplication with the
718+
# logic of gh_get_pr().
719+
#
704720
def gh_get_pr(self, *, url: str) -> Pr:
705721
out_str = self.cache_through(
706722
url,
@@ -711,7 +727,7 @@ class Main:
711727
"view",
712728
url,
713729
"--json",
714-
"number,title,body,baseRefName,headRefName,url,reviewDecision,state,labels",
730+
"number,title,body,baseRefName,headRefName,url,reviewDecision,state,labels,autoMergeRequest",
715731
],
716732
),
717733
)
@@ -725,111 +741,10 @@ class Main:
725741
url=value["url"],
726742
review_decision=value["reviewDecision"],
727743
state=value["state"],
728-
auto_merge_status="UNKNOWN",
744+
auto_merge_status="ENABLED" if value["autoMergeRequest"] else "DISABLED",
729745
labels=[label["name"] for label in value["labels"]],
730746
)
731747

732-
#
733-
# Sends a remote API request to bulk-load of existing PRs mentioned in the
734-
# provided list of commits. Returns PRs in the same order as commits. If no
735-
# PR URL is mentioned in commit message, returns None at its place.
736-
#
737-
def gh_get_prs(self, *, commits: list[Commit]) -> list[Pr | None]:
738-
@dataclass
739-
class Field:
740-
name: str
741-
url: str
742-
clause: str
743-
744-
fields: list[Field | None] = []
745-
for i, commit in enumerate(commits):
746-
if commit.url:
747-
m = re.match(r".*/(\d+)$", commit.url)
748-
if not m:
749-
raise UserException(
750-
f'Can\'t extract PR number from URL {commit.url} mentioned in the message of commit "{commit.title}"'
751-
)
752-
field_name = f"f{i}"
753-
pr_number = int(m.group(1))
754-
fields.append(
755-
Field(
756-
name=field_name,
757-
url=commit.url,
758-
clause=f" {field_name}: pullRequest(number: {pr_number}) "
759-
+ "{ "
760-
+ "number, title, body, baseRefName, headRefName, url, reviewDecision, state, "
761-
+ "autoMergeRequest { enabledAt }, "
762-
+ "labels(first: 100) { nodes { name } }"
763-
+ " }\n",
764-
)
765-
)
766-
else:
767-
fields.append(None)
768-
769-
clauses = [field.clause for field in fields if field]
770-
if not clauses:
771-
return [None for _ in commits]
772-
query = (
773-
"query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) {\n"
774-
+ "".join(clauses)
775-
+ "} }"
776-
)
777-
778-
(owner, name) = self.gh_get_current_repo_owner_and_name()
779-
out_str = self.shell(
780-
[
781-
"gh",
782-
"api",
783-
"graphql",
784-
"-F",
785-
f"owner={owner}",
786-
"-F",
787-
f"name={name}",
788-
"-f",
789-
f"query=\n{query}",
790-
],
791-
)
792-
out = json.loads(out_str)
793-
794-
repository = out.get("data", {}).get("repository", {})
795-
if not repository:
796-
raise UserException(
797-
f"Can't extract PR infos from GitHub GraphQL response:\n"
798-
+ json.dumps(out, indent=2)
799-
)
800-
801-
prs: list[Pr | None] = []
802-
for field in fields:
803-
value = field and repository.get(field.name)
804-
if value:
805-
prs.append(
806-
Pr(
807-
number=int(value["number"]),
808-
title=value["title"],
809-
body=value["body"],
810-
base_branch=re.sub(r"^refs/heads/", "", value["baseRefName"]),
811-
head_branch=re.sub(r"^refs/heads/", "", value["headRefName"]),
812-
url=value["url"],
813-
review_decision=value["reviewDecision"],
814-
state=value["state"],
815-
auto_merge_status=(
816-
"ENABLED" if value["autoMergeRequest"] else "UNKNOWN"
817-
),
818-
labels=(
819-
[
820-
label["name"]
821-
for label in value["labels"]["nodes"]
822-
if label
823-
]
824-
if value["labels"] and value["labels"]["nodes"]
825-
else []
826-
),
827-
)
828-
)
829-
else:
830-
prs.append(None)
831-
return prs
832-
833748
#
834749
# Returns parsed commits between two refs in reverse-chronological order
835750
# (newest commits first, oldest last, as they're shown in "git log").

0 commit comments

Comments
 (0)