upload: Allow clearing draft status with draft_on_create_only

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=<depth>**
 : Automatically mark PRs as drafts once they are this deep in a relative chain,

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

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)

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