From 106a22ce016a7ebb8967cfa95e47b8f36025efb7 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 17 Dec 2024 05:31:47 -0800 Subject: [PATCH] submit: Add --update-only flag For all submit commands, add an `--update-only` flag that will update existing open CRs but not create new ones. Resolves #516 --- .../unreleased/Added-20241220-211843.yaml | 5 ++ branch_submit.go | 36 ++++++++-- doc/includes/cli-reference.md | 23 +++++- doc/src/guide/cr.md | 47 ++++++++++++ testdata/script/submit_update_only.txt | 71 +++++++++++++++++++ 5 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 .changes/unreleased/Added-20241220-211843.yaml create mode 100644 testdata/script/submit_update_only.txt diff --git a/.changes/unreleased/Added-20241220-211843.yaml b/.changes/unreleased/Added-20241220-211843.yaml new file mode 100644 index 00000000..85ed18f9 --- /dev/null +++ b/.changes/unreleased/Added-20241220-211843.yaml @@ -0,0 +1,5 @@ +kind: Added +body: >- + submit: New `--update-only` flag tells submit commands to update open CRs but not create new ones. + Branches that would create new CRs are igonred. +time: 2024-12-20T21:18:43.773073-05:00 diff --git a/branch_submit.go b/branch_submit.go index bc19bf56..f8e0fab0 100644 --- a/branch_submit.go +++ b/branch_submit.go @@ -8,6 +8,7 @@ import ( "slices" "strconv" "strings" + "sync" "time" "github.com/charmbracelet/log" @@ -32,7 +33,8 @@ type submitOptions struct { NavigationComment navigationCommentWhen `name:"nav-comment" config:"submit.navigationComment" enum:"true,false,multiple" default:"true" help:"Whether to add a navigation comment to the change request. Must be one of: true, false, multiple."` - Force bool `help:"Force push, bypassing safety checks"` + Force bool `help:"Force push, bypassing safety checks"` + UpdateOnly bool `short:"u" help:"Only update existing change requests, do not create new ones"` // TODO: Other creation options e.g.: // - assignees @@ -47,8 +49,12 @@ For new Change Requests, a prompt will allow filling metadata. Use --fill to populate title and body from the commit messages, and --[no-]draft to set the draft status. Omitting the draft flag will leave the status unchanged of open CRs. + Use --no-publish to push branches without creating CRs. This has no effect if a branch already has an open CR. +Use --update-only to only update branches with existing CRs, +and skip those that would create new CRs. + Use --nav-comment=false to disable navigation comments in CRs, or --nav-comment=multiple to post those comments only if there are multiple CRs in the stack. ` @@ -79,8 +85,11 @@ func (*branchSubmitCmd) Help() string { use --draft/--no-draft to change its draft status. Without the flag, the draft status is not changed. - Use --no-publish to push the branch without creating a Change - Request. + Use --no-publish to push branches without creating CRs. + This has no effect if a branch already has an open CR. + Use --update-only to only update branches with existing CRs, + and skip those that would create new CRs. + Use --nav-comment=false to disable navigation comments in CRs, or --nav-comment=multiple to post those comments only if there are multiple CRs in the stack. `) @@ -153,8 +162,14 @@ func (cmd *branchSubmitCmd) run( } } - if !cmd.DryRun && cmd.Publish { - session.branches = append(session.branches, cmd.Branch) + // Various code paths down below should call this + // if the branch is being published as a CR (new or existing) + // so it should get a nav comment. + var _needsNavCommentOnce sync.Once + needsNavComment := func() { + _needsNavCommentOnce.Do(func() { + session.branches = append(session.branches, cmd.Branch) + }) } commitHash, err := repo.PeelToCommit(ctx, cmd.Branch) @@ -356,6 +371,14 @@ func (cmd *branchSubmitCmd) run( upstreamBranch = unique } + if cmd.UpdateOnly { + if !cmd.DryRun { + // TODO: config to disable this message? + log.Infof("%v: Skipping unsubmitted branch: --update-only", cmd.Branch) + } + return nil + } + if cmd.DryRun { if cmd.Publish { log.Infof("WOULD create a CR for %s", cmd.Branch) @@ -367,6 +390,8 @@ func (cmd *branchSubmitCmd) run( var prepared *preparedBranch if cmd.Publish { + needsNavComment() + remoteRepo, err := session.RemoteRepo.Get(ctx) if err != nil { return fmt.Errorf("prepare publish: %w", err) @@ -469,6 +494,7 @@ func (cmd *branchSubmitCmd) run( log.Infof("Pushed %s", cmd.Branch) } } else { + needsNavComment() must.NotBeBlankf(upstreamBranch, "upstream branch must be set if branch has a CR") if !cmd.Publish { diff --git a/doc/includes/cli-reference.md b/doc/includes/cli-reference.md index 85576ad9..dfc993fa 100644 --- a/doc/includes/cli-reference.md +++ b/doc/includes/cli-reference.md @@ -202,8 +202,12 @@ For new Change Requests, a prompt will allow filling metadata. Use --fill to populate title and body from the commit messages, and --[no-]draft to set the draft status. Omitting the draft flag will leave the status unchanged of open CRs. + Use --no-publish to push branches without creating CRs. This has no effect if a branch already has an open CR. +Use --update-only to only update branches with existing CRs, +and skip those that would create new CRs. + Use --nav-comment=false to disable navigation comments in CRs, or --nav-comment=multiple to post those comments only if there are multiple CRs in the stack. @@ -217,6 +221,7 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs * `-w`, `--[no-]web` ([:material-wrench:{ .middle title="spice.submit.web" }](/cli/config.md#spicesubmitweb)): Open submitted changes in a web browser * `--nav-comment=true` ([:material-wrench:{ .middle title="spice.submit.navigationComment" }](/cli/config.md#spicesubmitnavigationcomment)): Whether to add a navigation comment to the change request. Must be one of: true, false, multiple. * `--force`: Force push, bypassing safety checks +* `-u`, `--update-only`: Only update existing change requests, do not create new ones **Configuration**: [spice.submit.publish](/cli/config.md#spicesubmitpublish), [spice.submit.web](/cli/config.md#spicesubmitweb), [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment) @@ -275,8 +280,12 @@ For new Change Requests, a prompt will allow filling metadata. Use --fill to populate title and body from the commit messages, and --[no-]draft to set the draft status. Omitting the draft flag will leave the status unchanged of open CRs. + Use --no-publish to push branches without creating CRs. This has no effect if a branch already has an open CR. +Use --update-only to only update branches with existing CRs, +and skip those that would create new CRs. + Use --nav-comment=false to disable navigation comments in CRs, or --nav-comment=multiple to post those comments only if there are multiple CRs in the stack. @@ -290,6 +299,7 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs * `-w`, `--[no-]web` ([:material-wrench:{ .middle title="spice.submit.web" }](/cli/config.md#spicesubmitweb)): Open submitted changes in a web browser * `--nav-comment=true` ([:material-wrench:{ .middle title="spice.submit.navigationComment" }](/cli/config.md#spicesubmitnavigationcomment)): Whether to add a navigation comment to the change request. Must be one of: true, false, multiple. * `--force`: Force push, bypassing safety checks +* `-u`, `--update-only`: Only update existing change requests, do not create new ones * `--branch=NAME`: Branch to start at **Configuration**: [spice.submit.publish](/cli/config.md#spicesubmitpublish), [spice.submit.web](/cli/config.md#spicesubmitweb), [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment) @@ -368,8 +378,12 @@ For new Change Requests, a prompt will allow filling metadata. Use --fill to populate title and body from the commit messages, and --[no-]draft to set the draft status. Omitting the draft flag will leave the status unchanged of open CRs. + Use --no-publish to push branches without creating CRs. This has no effect if a branch already has an open CR. +Use --update-only to only update branches with existing CRs, +and skip those that would create new CRs. + Use --nav-comment=false to disable navigation comments in CRs, or --nav-comment=multiple to post those comments only if there are multiple CRs in the stack. @@ -383,6 +397,7 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs * `-w`, `--[no-]web` ([:material-wrench:{ .middle title="spice.submit.web" }](/cli/config.md#spicesubmitweb)): Open submitted changes in a web browser * `--nav-comment=true` ([:material-wrench:{ .middle title="spice.submit.navigationComment" }](/cli/config.md#spicesubmitnavigationcomment)): Whether to add a navigation comment to the change request. Must be one of: true, false, multiple. * `--force`: Force push, bypassing safety checks +* `-u`, `--update-only`: Only update existing change requests, do not create new ones * `--branch=NAME`: Branch to start at **Configuration**: [spice.submit.publish](/cli/config.md#spicesubmitpublish), [spice.submit.web](/cli/config.md#spicesubmitweb), [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment) @@ -739,8 +754,11 @@ For updating Change Requests, use --draft/--no-draft to change its draft status. Without the flag, the draft status is not changed. -Use --no-publish to push the branch without creating a Change -Request. +Use --no-publish to push branches without creating CRs. +This has no effect if a branch already has an open CR. +Use --update-only to only update branches with existing CRs, +and skip those that would create new CRs. + Use --nav-comment=false to disable navigation comments in CRs, or --nav-comment=multiple to post those comments only if there are multiple CRs in the stack. @@ -753,6 +771,7 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs * `-w`, `--[no-]web` ([:material-wrench:{ .middle title="spice.submit.web" }](/cli/config.md#spicesubmitweb)): Open submitted changes in a web browser * `--nav-comment=true` ([:material-wrench:{ .middle title="spice.submit.navigationComment" }](/cli/config.md#spicesubmitnavigationcomment)): Whether to add a navigation comment to the change request. Must be one of: true, false, multiple. * `--force`: Force push, bypassing safety checks +* `-u`, `--update-only`: Only update existing change requests, do not create new ones * `--title=TITLE`: Title of the change request * `--body=BODY`: Body of the change request * `--branch=NAME`: Branch to submit diff --git a/doc/src/guide/cr.md b/doc/src/guide/cr.md index 5cb966c4..c5aa4565 100644 --- a/doc/src/guide/cr.md +++ b/doc/src/guide/cr.md @@ -135,6 +135,53 @@ if the operation could result in data loss. To override these safety checks and push to a branch anyway, use the `--force` flag. +### Update existing CRs only + + + +All submit commands support the `--update-only` flag. +If provided, the submission will update existing CRs in a stack, +but not create new ones. + + +This is most convenient with $$gs stack submit$$ and friends, +allowing you to iterate on a local change that isn't ready for submission, +while still being able to pull updates into downstack PRs +that have already been submitted. + +??? example "Example workflow" + + Suppose we're starting with a stack: + + main -> bird (#1) -> fish (#2) -> goat + + `bird` and `fish` have already been submitted, `goat` is in-progress. + + ```freeze language="terminal" + {gray}# While working in goat, make a minor fixup to bird.{reset} + {yellow}[goat]{reset} {green}${reset} gs commit create -m {mag}'bird: preen a little'{reset} + + {gray}# Pull that change into bird{reset} + {yellow}[goat]{reset} {green}${reset} gs branch checkout bird + {yellow}[bird]{reset} {green}${reset} git restore --source {cyan}$(gs top -n){reset} -- bird.go + {yellow}[bird]{reset} {green}${reset} gs commit create -a -m {mag}'preen a little'{reset} + {green}INF{reset} fish: restacked on bird + {green}INF{reset} goat: restacked on fish + + {gray}# Update fish and bird in one command without submitting goat{reset} + {yellow}[bird]{reset} {green}${reset} gs stack submit --update-only + {green}INF{reset} bird: Updated #1 + {green}INF{reset} goat: Updated #2 + {green}INF{reset} goat: Skipping unsubmitted branch: --update-only + ``` + + !!! tip + + The above example makes use of the `-n`/`--dry-run` flag + of the [stack navigation commands](branch.md#navigating-the-stack). + With this flag, the command prints the hash of the target branch + without checking it out. + ## Syncing with upstream To sync with the upstream repository, diff --git a/testdata/script/submit_update_only.txt b/testdata/script/submit_update_only.txt new file mode 100644 index 00000000..39260cb4 --- /dev/null +++ b/testdata/script/submit_update_only.txt @@ -0,0 +1,71 @@ +# various submit commands with the --update-only flag + +as 'Test ' +at '2024-12-20T21:00:00Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +# set up a fake GitHub remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# start with stack: main -> feat1 -> feat2 +git add feat1.txt +gs bc -m feat1 +git add feat2.txt +gs bc -m feat2 + +# submit: nothing to do +gs ss --update-only --fill +stderr 'feat1: Skipping unsubmitted branch' +stderr 'feat2: Skipping unsubmitted branch' +gs down +gs bs --update-only --fill +stderr 'feat1: Skipping unsubmitted branch' + +# submit feat1 +gs bco feat1 +gs bs --fill +stderr 'Created #1' + +# modify both branches +cp $WORK/extra/feat1-new.txt feat1.txt +git add feat1.txt +gs cc -m 'update feat1' +gs up +cp $WORK/extra/feat2-new.txt feat2.txt +git add feat2.txt +gs cc -m 'update feat2' + +# submit: only feat1 is submitted +gs downstack submit --update-only +stderr 'Updated #1' +stderr 'feat2: Skipping unsubmitted branch' + +gs ll +cmp stderr $WORK/golden/ll-after.txt + +-- repo/feat1.txt -- +feature 1 +-- repo/feat2.txt -- +feature 2 +-- extra/feat1-new.txt -- +feature 1 new +-- extra/feat2-new.txt -- +feature 2 new +-- golden/ll-after.txt -- + ┏━■ feat2 ◀ + ┃ ec789dc update feat2 (now) + ┃ f76d5f5 feat2 (now) +┏━┻□ feat1 (#1) +┃ f879555 update feat1 (now) +┃ 4ea4a27 feat1 (now) +main