Skip to content

Commit

Permalink
fix: don't recreate merged PR (#500)
Browse files Browse the repository at this point in the history
If skip-rebase is used, and first PR of the stack is merged, we recreate
it because the pull request has vanish remotely.

This doesn't make sense, this change instead mark it locally as merged.

The merged status will displayed in cli output until the next rebase.

This avoids creating useless PR.
  • Loading branch information
sileht authored Oct 22, 2024
1 parent 4265fd6 commit aca10dd
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 24 deletions.
55 changes: 34 additions & 21 deletions mergify_cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
"draft": True,
"state": "",
"head": {"sha": ""},
"merged_at": None,
"merge_commit_sha": None,
},
)

Expand Down Expand Up @@ -166,24 +168,28 @@ async def get_changeid_and_pull(
) -> tuple[ChangeId, github_types.PullRequest | None]:
branch = ref["ref"][len("refs/heads/") :]
changeid = ChangeId(branch[len(stack_prefix) + 1 :])
r = await client.get("pulls", params={"head": f"{user}:{branch}", "state": "open"})
r = await client.get("pulls", params={"head": f"{user}:{branch}", "state": "all"})
check_for_status(r)
pulls = [
p
for p in typing.cast(list[github_types.PullRequest], r.json())
if p["state"] == "open"
]
if len(pulls) > 1:
msg = f"More than 1 pull found with this head: {branch}"
raise RuntimeError(msg)
if pulls:
pull = pulls[0]
if pull["body"] is None:
r = await client.get(f"pulls/{pull['number']}")
check_for_status(r)
pull = typing.cast(github_types.PullRequest, r.json())
return changeid, pull
return changeid, None
pulls = typing.cast(list[github_types.PullRequest], r.json())
opened_pulls = [pull for pull in pulls if pull["state"] == "open"]
merged_pulls = [pull for pull in pulls if pull["merged_at"] is not None]

if opened_pulls:
if len(opened_pulls) > 1:
msg = f"More than 1 pull found with this head: {branch}"
raise RuntimeError(msg)

pull = opened_pulls[0]
elif merged_pulls:
pull = merged_pulls[0]
else:
return changeid, None

if pull["body"] is None or "merged_at" not in pull:
r = await client.get(f"pulls/{pull['number']}")
check_for_status(r)
pull = typing.cast(github_types.PullRequest, r.json())
return changeid, pull


Change = typing.NewType("Change", tuple[ChangeId, str, str, str])
Expand Down Expand Up @@ -230,11 +236,11 @@ async def get_local_changes( # noqa: PLR0913,PLR0917

url: str = ""
draft: str = ""
merged: str = ""
action: str
commit_info: str
if pull is None:
commit_info = commit[-7:]

if only_update_existing_pulls:
action = "nothing (to create, only updating)"
else:
Expand All @@ -244,6 +250,10 @@ async def get_local_changes( # noqa: PLR0913,PLR0917
if create_as_draft:
draft = " [yellow](draft)[/]"

elif pull["merged_at"]:
merged = " [purple](merged)[/]"
action = "nothing"
commit_info = f"{pull['merge_commit_sha']}"
else:
url = pull["html_url"]
head_commit = commit[-7:]
Expand All @@ -258,9 +268,7 @@ async def get_local_changes( # noqa: PLR0913,PLR0917
if pull["draft"]:
draft = " [yellow](draft)[/]"

log_message = (
f"* [yellow]\\[{action}][/] '[red]{commit_info}[/] - [b]{title}[/]{draft}"
)
log_message = f"* [yellow]\\[{action}][/] '[red]{commit_info}[/] - [b]{title}[/]{draft}{merged}"
if url:
log_message += f" {url}"

Expand Down Expand Up @@ -297,6 +305,9 @@ async def create_or_update_comments(
stack_comment = StackComment(pulls)

for pull in pulls:
if pull["merged_at"]:
continue

new_body = stack_comment.body(pull)

r = await client.get(f"issues/{pull['number']}/comments")
Expand Down Expand Up @@ -678,6 +689,8 @@ async def stack_push( # noqa: PLR0913, PLR0914, PLR0915, PLR0917, PLR0912
log_message += f" - [b]{pull['title']}[/]"
if pull["draft"]:
log_message += " [yellow](draft)[/]"
if pull["merged_at"]:
log_message += " [purple](merged)[/]"

log_message += f" {pull['html_url']}"

Expand Down
2 changes: 2 additions & 0 deletions mergify_cli/github_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class PullRequest(typing.TypedDict):
state: str
draft: bool
node_id: str
merged_at: str | None
merge_commit_sha: str | None


class Comment(typing.TypedDict):
Expand Down
12 changes: 9 additions & 3 deletions mergify_cli/tests/test_mergify_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ async def test_stack_create(
"title": "Title commit 1",
"head": {"sha": "commit1_sha"},
"state": "open",
"merged_at": None,
"draft": False,
"node_id": "",
},
Expand All @@ -195,6 +196,7 @@ async def test_stack_create(
"title": "Title commit 2",
"head": {"sha": "commit2_sha"},
"state": "open",
"merged_at": None,
"draft": False,
"node_id": "",
},
Expand Down Expand Up @@ -290,6 +292,7 @@ async def test_stack_create_single_pull(
"title": "Title commit 1",
"head": {"sha": "commit1_sha"},
"state": "open",
"merged_at": None,
"draft": False,
"node_id": "",
},
Expand Down Expand Up @@ -343,7 +346,7 @@ async def test_stack_update_no_rebase(
],
)
respx_mock.get(
"/repos/user/repo/pulls?head=user:/current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50&state=open",
"/repos/user/repo/pulls?head=user:/current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50&state=all",
).respond(
200,
json=[
Expand All @@ -354,6 +357,7 @@ async def test_stack_update_no_rebase(
"head": {"sha": "previous_commit_sha"},
"body": "body",
"state": "open",
"merged_at": None,
"draft": False,
"node_id": "",
},
Expand Down Expand Up @@ -421,7 +425,7 @@ async def test_stack_update(
],
)
respx_mock.get(
"/repos/user/repo/pulls?head=user:/current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50&state=open",
"/repos/user/repo/pulls?head=user:/current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50&state=all",
).respond(
200,
json=[
Expand All @@ -432,6 +436,7 @@ async def test_stack_update(
"head": {"sha": "previous_commit_sha"},
"body": "body",
"state": "open",
"merged_at": None,
"draft": False,
"node_id": "",
},
Expand Down Expand Up @@ -499,7 +504,7 @@ async def test_stack_update_keep_title_and_body(
],
)
respx_mock.get(
"/repos/user/repo/pulls?head=user:/current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50&state=open",
"/repos/user/repo/pulls?head=user:/current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50&state=all",
).respond(
200,
json=[
Expand All @@ -509,6 +514,7 @@ async def test_stack_update_keep_title_and_body(
"title": "Title",
"head": {"sha": "previous_commit_sha"},
"state": "open",
"merged_at": None,
"draft": False,
"node_id": "",
"body": "DONT TOUCH ME\n\nDepends-On: #12345\n",
Expand Down

0 comments on commit aca10dd

Please sign in to comment.