Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upload: Query head oid of a PR via commits #206

Merged
merged 1 commit into from
Jan 2, 2025
Merged

upload: Query head oid of a PR via commits #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions revup/github_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class PrInfo:

baseRef: str
headRef: str
baseRefOid: GitCommitHash
headRefOid: GitCommitHash
baseRefOid: Optional[GitCommitHash]
headRefOid: Optional[GitCommitHash]
body: str
title: str
id: str = ""
Expand Down Expand Up @@ -181,12 +181,10 @@ async def query_everything(
state
url
baseRefName
headRefOid
body
title
isDraft
updatedAt
commits (first: 1) {{
baseCommit: commits(first: 1) {{
nodes {{
commit {{
parents (first: 1) {{
Expand All @@ -197,6 +195,13 @@ async def query_everything(
}}
}}
}}
headCommit: commits(last: 1) {{
nodes {{
commit {{
oid
}}
}}
}}
reviewRequests (first: 25) {{
nodes {{
requestedReviewer {{
Expand Down Expand Up @@ -279,15 +284,20 @@ async def query_everything(
for user in this_node["assignees"]["nodes"]:
assignees.add(user["login"])
assignee_ids.add(user["id"])
headRefOid = this_node["headRefOid"]
# Github's "baseRefOid" in the api field returns the ToT commit for the base ref
# which isn't what we want, since that commit may not exist locally. Instead
# we get the parent of the first commit, which is the base ref it was actually
# uploaded against.

# The plain headRef and baseRef fields return the latest commit id associated with
# that branch name which may be newer than the PR itself if it was merged. We want
# the ids of the commits actually last associated with the PR, which we query from
# the commit list. This can also mean they are None if the PR has 0 commits.
headRefOid = (
this_node["headCommit"]["nodes"][0]["commit"]["oid"]
if this_node["headCommit"]["nodes"]
else None
)
baseRefOid = (
headRefOid
if not this_node["commits"]["nodes"]
else this_node["commits"]["nodes"][0]["commit"]["parents"]["nodes"][0]["oid"]
this_node["baseCommit"]["nodes"][0]["commit"]["parents"]["nodes"][0]["oid"]
if this_node["baseCommit"]["nodes"]
else None
)

comments = []
Expand Down
17 changes: 15 additions & 2 deletions revup/topic_stack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,8 @@ async def create_patchsets_comment(
or not self.repo_info
or not self.fork_info
or review.pr_info is None
or review.pr_info.headRefOid is None
or review.pr_info.baseRefOid is None
or review.base_ref is None
):
return None
Expand Down Expand Up @@ -748,6 +750,12 @@ async def mark_rebases(self, skip_rebase: bool) -> None:
# but they are both corner cases and the worst that could happen is we fail to
# recreate the pr (but warn anyway).
review.status = PrStatus.NEW
review.pr_info = None

if review.pr_info and (not review.pr_info.baseRefOid or not review.pr_info.headRefOid):
logging.warning(f"Branch {review.remote_head} was merged but has no commits!")
review.status = PrStatus.NEW
review.pr_info = None

if review.pr_info is not None and review.remote_base != review.pr_info.baseRef:
logging.debug(
Expand All @@ -766,6 +774,9 @@ async def mark_rebases(self, skip_rebase: bool) -> None:
# This is a new pr, no need to check patch ids
review.is_pure_rebase = False
else:
assert (
review.pr_info.baseRefOid is not None and review.pr_info.headRefOid is not None
)
if not topic.patch_ids:
# Lazily load patch ids for the topic.
topic.patch_ids = await asyncio.gather(
Expand Down Expand Up @@ -993,8 +1004,10 @@ async def fetch_git_refs(self) -> None:

to_fetch = set()
for _, _, _, review in self.all_reviews_iter():
if review.pr_info is not None and not await self.git_ctx.is_branch_or_commit(
review.pr_info.headRefOid
if (
review.pr_info is not None
and review.pr_info.headRefOid is not None
and not await self.git_ctx.is_branch_or_commit(review.pr_info.headRefOid)
):
to_fetch.add(review.pr_info.headRefOid)

Expand Down
Loading