diff --git a/docs/upload.md b/docs/upload.md index fdb1464..f825e3d 100644 --- a/docs/upload.md +++ b/docs/upload.md @@ -248,9 +248,8 @@ using an empty commit purely for PR title and body text without that commit appe the merged history. **--draft-on-create-only** -: Only apply draft status when creating a PR, and never change it afterwards. -This allows marking a PR ready for review in github without revup turning it -back into a draft. +: Only apply draft status when creating a PR, not afterwards. This allows marking a PR +ready for review in github without revup turning it back into a draft. **--deep-stack-draft=** : Automatically mark PRs as drafts once they are this deep in a relative chain, diff --git a/revup/topic_stack.py b/revup/topic_stack.py index 740dc06..245ef32 100644 --- a/revup/topic_stack.py +++ b/revup/topic_stack.py @@ -1237,7 +1237,7 @@ async def push_git_refs(self, uploader: str, create_local_branches: bool) -> Non ), ) - async def query(self) -> None: + async def query(self, draft_on_create_only: bool = False) -> None: """ Query pr and reviewer/label info from the forge """ @@ -1295,8 +1295,14 @@ async def query(self) -> None: review.pr_info = prs[i] if review.pr_info is None: review.status = PrStatus.NEW - elif review.pr_info.state == "MERGED": - review.status = PrStatus.MERGED + else: + if review.pr_info.state == "MERGED": + review.status = PrStatus.MERGED + if draft_on_create_only and not review.pr_info.is_draft: + # Draft status can only be cleared after creation, never reapplied, since + # the user may have marked the pr ready in the forge. Resolve it here so + # that status output shows the state the pr will actually be left in. + review.is_draft = False i += 1 while i < len(pr_targets): @@ -1341,7 +1347,6 @@ def populate_update_info( update_pr_body_arg: bool, force_reviewers: bool = False, pr_body_source: PrBodySource = PrBodySource.FIRST_COMMIT, - draft_on_create_only: bool = False, ) -> None: """ Populate information necessary to do PR creation / update on the forge. @@ -1466,10 +1471,7 @@ def populate_update_info( review.pr_update.body = body if update_pr_body and review.pr_info.title != title: review.pr_update.title = title - if draft_on_create_only: - # The forge's draft status wins, since the user may have changed it there - review.is_draft = review.pr_info.is_draft - elif review.pr_info.is_draft != review.is_draft: + if review.pr_info.is_draft != review.is_draft: review.pr_update.is_draft = review.is_draft review.pr_update.label_ids = label_ids review.pr_update.reviewer_ids = reviewer_ids diff --git a/revup/upload.py b/revup/upload.py index c116cc9..7aac4c4 100644 --- a/revup/upload.py +++ b/revup/upload.py @@ -100,7 +100,7 @@ async def run( if not args.dry_run and not args.push_only: with get_console().status(f"Querying {forge.name}…"): - await topics.query() + await topics.query(args.draft_on_create_only) await topics.fetch_git_refs() await topics.mark_rebases(not args.rebase) @@ -126,7 +126,6 @@ async def run( args.update_pr_body, args.force_reviewers, args.pr_body_source, - args.draft_on_create_only, ) if not args.skip_confirm and topics.num_reviews_changed() > 0: topics.print(not args.verbose) diff --git a/tests/test_upload.py b/tests/test_upload.py index a0e54f0..51049bc 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -1860,6 +1860,44 @@ async def test_draft_on_create_only_keeps_pr_undrafted(self): assert review.is_draft is False assert pr.is_draft is False + @async_test + async def test_draft_on_create_only_status_shows_forge_draft(self): + """Status shows the draft state the pr will be left in, not the one revup wanted.""" + async with GitTestEnvironment() as env: + await setup_repo(env) + forge = FakeForge() + await env.commit("feat\n\nTopic: alpha\nDraft: true", {"a.txt": "a"}) + + await full_upload_pipeline(env, forge, draft_on_create_only=True) + pr = list(forge.prs.values())[0] + pr.is_draft = False + + topics = await full_upload_pipeline(env, forge, draft_on_create_only=True, status=True) + + assert topics.topics["alpha"].reviews["origin/main"].is_draft is False + + @async_test + async def test_draft_on_create_only_clears_draft_when_no_longer_needed(self): + """A pr revup drafted for being deep is marked ready once it leaves that depth.""" + async with GitTestEnvironment() as env: + await setup_repo(env) + forge = FakeForge() + await env.commit("first\n\nTopic: first", {"a.txt": "a"}) + await env.commit("second\n\nTopic: second\nRelative: first", {"b.txt": "b"}) + + await full_upload_pipeline(env, forge, deep_stack_draft=2, draft_on_create_only=True) + pr = [p for p in forge.created_prs if p.headRef.endswith("second")][0] + assert pr.is_draft is True + + # Make second no longer relative, which drops it to depth 1 + await env.git_ctx.git("commit", "--amend", "-m", "second\n\nTopic: second") + topics = await full_upload_pipeline( + env, forge, deep_stack_draft=2, draft_on_create_only=True + ) + + assert topics.topics["second"].reviews["origin/main"].pr_update.is_draft is False + assert pr.is_draft is False + class TestForgeReviewGraph: @async_test