submit: support --publish opt-in mode

Change --no-publish to --publish, defaulting to true,
with a --no-publish opt-out.
Support setting the default value via configuration,
allowing users to always opt out of creating CRs,
unless the --publish flag is added to the command.

This will make the experience of using submit with unsupported forges
smoother.

.changes/unreleased/Added-20240821-213515.yaml

@@ -0,0 +1,6 @@
+kind: Added
+body: >-
+  {branch, downstack, upstack, stack} submit:
+  Support changing the default for --no-publish with the 'spice.submit.publish' configuration.
+  If the configuration is set to false, use the --publish flag to opt-in to publishing.
+time: 2024-08-21T21:35:15.287687-07:00

branch_submit.go

@@ -23,8 +23,8 @@ type submitOptions struct {
 	DryRun bool `short:"n" help:"Don't actually submit the stack"`
 	Fill   bool `short:"c" help:"Fill in the change title and body from the commit messages"`
 	// TODO: Default to Fill if --no-prompt?
-	Draft     *bool `negatable:"" help:"Whether to mark change requests as drafts"`
-	NoPublish bool  `name:"no-publish" help:"Push branches but don't create change requests"`
+	Draft   *bool `negatable:"" help:"Whether to mark change requests as drafts"`
+	Publish bool  `name:"publish" negatable:"" default:"true" config:"submit.publish" help:"Whether to create CRs for pushed branches. Defaults to true."`
 
 	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."`
 
@@ -147,7 +147,7 @@ func (cmd *branchSubmitCmd) run(
 		}
 	}
 
-	if !cmd.DryRun && !cmd.NoPublish {
+	if !cmd.DryRun && cmd.Publish {
 		session.branches = append(session.branches, cmd.Branch)
 	}
 
@@ -170,7 +170,7 @@ func (cmd *branchSubmitCmd) run(
 	}
 
 	var existingChange *forge.FindChangeItem
-	if branch.Change == nil && !cmd.NoPublish {
+	if branch.Change == nil && cmd.Publish {
 		// If the branch doesn't have a CR associated with it,
 		// we'll probably need to create one,
 		// but verify that there isn't already one open.
@@ -251,16 +251,16 @@ func (cmd *branchSubmitCmd) run(
 	// At this point, existingChange is nil only if we need to create a new CR.
 	if existingChange == nil {
 		if cmd.DryRun {
-			if cmd.NoPublish {
-				log.Infof("WOULD push branch %s", cmd.Branch)
-			} else {
+			if cmd.Publish {
 				log.Infof("WOULD create a CR for %s", cmd.Branch)
+			} else {
+				log.Infof("WOULD push branch %s", cmd.Branch)
 			}
 			return nil
 		}
 
 		var prepared *preparedBranch
-		if !cmd.NoPublish {
+		if cmd.Publish {
 			remoteRepo, err := session.RemoteRepo.Get(ctx)
 			if err != nil {
 				return fmt.Errorf("prepare publish: %w", err)
@@ -354,7 +354,7 @@ func (cmd *branchSubmitCmd) run(
 			log.Infof("Pushed %s", cmd.Branch)
 		}
 	} else {
-		if cmd.NoPublish {
+		if !cmd.Publish {
 			log.Warnf("Ignoring --no-publish: %s was already published: %s", cmd.Branch, existingChange.URL)
 		}
 

doc/includes/cli-reference.md

@@ -203,11 +203,11 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs
 * `-n`, `--dry-run`: Don't actually submit the stack
 * `-c`, `--fill`: Fill in the change title and body from the commit messages
 * `--[no-]draft`: Whether to mark change requests as drafts
-* `--no-publish`: Push branches but don't create change requests
+* `--[no-]publish` ([:material-wrench:{ .middle title="spice.submit.publish" }](/cli/config.md#spicesubmitpublish)): Whether to create CRs for pushed branches. Defaults to true.
 * `--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
 
-**Configuration**: [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
+**Configuration**: [spice.submit.publish](/cli/config.md#spicesubmitpublish), [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
 
 ### gs stack restack
 
@@ -275,12 +275,12 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs
 * `-n`, `--dry-run`: Don't actually submit the stack
 * `-c`, `--fill`: Fill in the change title and body from the commit messages
 * `--[no-]draft`: Whether to mark change requests as drafts
-* `--no-publish`: Push branches but don't create change requests
+* `--[no-]publish` ([:material-wrench:{ .middle title="spice.submit.publish" }](/cli/config.md#spicesubmitpublish)): Whether to create CRs for pushed branches. Defaults to true.
 * `--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
 * `--branch=NAME`: Branch to start at
 
-**Configuration**: [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
+**Configuration**: [spice.submit.publish](/cli/config.md#spicesubmitpublish), [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
 
 ### gs upstack restack
 
@@ -367,12 +367,12 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs
 * `-n`, `--dry-run`: Don't actually submit the stack
 * `-c`, `--fill`: Fill in the change title and body from the commit messages
 * `--[no-]draft`: Whether to mark change requests as drafts
-* `--no-publish`: Push branches but don't create change requests
+* `--[no-]publish` ([:material-wrench:{ .middle title="spice.submit.publish" }](/cli/config.md#spicesubmitpublish)): Whether to create CRs for pushed branches. Defaults to true.
 * `--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
 * `--branch=NAME`: Branch to start at
 
-**Configuration**: [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
+**Configuration**: [spice.submit.publish](/cli/config.md#spicesubmitpublish), [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
 
 ### gs downstack edit
 
@@ -734,14 +734,14 @@ or --nav-comment=multiple to post those comments only if there are multiple CRs
 * `-n`, `--dry-run`: Don't actually submit the stack
 * `-c`, `--fill`: Fill in the change title and body from the commit messages
 * `--[no-]draft`: Whether to mark change requests as drafts
-* `--no-publish`: Push branches but don't create change requests
+* `--[no-]publish` ([:material-wrench:{ .middle title="spice.submit.publish" }](/cli/config.md#spicesubmitpublish)): Whether to create CRs for pushed branches. Defaults to true.
 * `--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
 * `--title=TITLE`: Title of the change request
 * `--body=BODY`: Body of the change request
 * `--branch=NAME`: Branch to submit
 
-**Configuration**: [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
+**Configuration**: [spice.submit.publish](/cli/config.md#spicesubmitpublish), [spice.submit.navigationComment](/cli/config.md#spicesubmitnavigationcomment)
 
 ## Commit
 

doc/src/cli/config.md

@@ -90,3 +90,20 @@ should post or update a navigation comment to the CR.
 - `false`: don't post or update navigation comments
 - `multiple`:
   post or update navigation comments only for stacks with at least two CRs
+
+### spice.submit.publish
+
+<!-- gs:version unreleased -->
+
+Whether submission commands ($$gs branch submit$$ and friends)
+should publish a CR to the forge.
+
+If this is set to false, submit commands will push branches,
+but not create CRs.
+In that case, the `--publish` flag will opt-in to creating CRs
+on a case-by-case basis.
+
+**Accepted values:**
+
+- `true` (default)
+- `false`

testdata/script/branch_submit_config_no_publish.txt

@@ -0,0 +1,83 @@
+# branch submit supports spice.submit.publish configuration
+# to disable publishing until --publish is specified.
+
+as 'Test <[email protected]>'
+at '2024-08-21T21:29:32Z'
+
+# 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
+
+# opt out of publishing
+git config spice.submit.publish false
+
+git add feature1.txt
+gs bc -m feature1
+gs branch submit
+
+# branch submit doesn't publish
+shamhub dump changes
+cmp stdout $WORK/golden/no-pulls.txt
+
+mv feature1-v2.txt feature1.txt
+git add feature1.txt
+gs cc -m feature1-v2
+gs branch submit
+
+# branch submit doesn't publish
+shamhub dump changes
+cmp stdout $WORK/golden/no-pulls.txt
+
+# publish
+mv feature1-v3.txt feature1.txt
+git add feature1.txt
+gs ca -n -m 'feature1 v3'
+gs branch submit --publish --fill
+
+gs ll
+cmp stderr $WORK/golden/log-long.txt
+
+shamhub dump changes
+cmpenv stdout $WORK/golden/post-publish.txt
+
+-- repo/feature1.txt --
+feature 1
+-- repo/feature1-v2.txt --
+feature 1 v2
+-- repo/feature1-v3.txt --
+feature 1 v3
+-- golden/no-pulls.txt --
+[]
+-- golden/log-long.txt --
+┏━■ feature1 (#1) ◀
+┃   5a09fe1 feature1 v3 (now)
+┃   293742b feature1 (now)
+main
+-- golden/post-publish.txt --
+[
+  {
+    "number": 1,
+    "html_url": "$SHAMHUB_URL/alice/example/change/1",
+    "state": "open",
+    "title": "feature1",
+    "body": "feature1\n\nfeature1 v3",
+    "base": {
+      "ref": "main",
+      "sha": "b709fee62def5240cbbf662a05fdad097ab30e20"
+    },
+    "head": {
+      "ref": "feature1",
+      "sha": "5a09fe1c62b68c6aad1c811929ba69d591f3babb"
+    }
+  }
+]

upstack_submit.go

@@ -57,7 +57,7 @@ func (cmd *upstackSubmitCmd) Run(
 				return fmt.Errorf("lookup base %v: %w", b.Base, err)
 			}
 
-			if base.Change == nil && !cmd.NoPublish {
+			if base.Change == nil && cmd.Publish {
 				log.Errorf("%v: base (%v) has not been submitted", cmd.Branch, b.Base)
 				return errors.New("submit the base branch first")
 			}