update default upstream when forking repo during PR creation by daviddl9 · Pull Request #10458 · cli/cli

daviddl9 · 2025-02-17T03:11:53Z

This PR updates the behaviour to set the default upstream when forking a repo during PR creation. It ensures that the behaviour when performing a fork during PR creation remains consistent with gh repo fork.

Closes #10277

This bash script can be used to verify these changes:

#!/bin/bash
set -e

# Configuration
UPSTREAM_REPO="BagToad/testing-public-1"
GITHUB_HOST="github.com"
GH_BIN="/Users/davidlivingston/Work/cli/bin/gh" # update with your own path

# Function to cleanup on exit or error
cleanup() {
    local exit_code=$?
    echo "Performing cleanup..."
    
    # Clean up local directory
    if [ -n "$TEST_DIR" ] && [ -d "$TEST_DIR" ]; then
        echo "Removing local test directory..."
        rm -rf "$TEST_DIR"
    fi

    # Delete fork if it exists
    if [ -n "$FORK_EXISTS" ]; then
        echo "Deleting forked repository..."
        if "$GH_BIN" repo delete "$FORK_OWNER/$FORK_NAME" --confirm; then
            echo "Successfully deleted fork"
        else
            echo "Failed to delete fork"
            exit 1
        fi
    fi

    exit $exit_code
}

# Set up cleanup trap
trap cleanup EXIT

# Get the authenticated user
FORK_OWNER=$("$GH_BIN" api user -q .login)
FORK_NAME="testing-public-1"
echo "Authenticated as: $FORK_OWNER"

# Check if fork already exists
if "$GH_BIN" repo view "$FORK_OWNER/$FORK_NAME" &>/dev/null; then
    echo "Fork already exists at $FORK_OWNER/$FORK_NAME"
    echo "Please delete it first or use a different test account"
    exit 1
fi

# Test setup
echo "Setting up test environment..."

# Create a new directory for testing
TEST_DIR=$(mktemp -d)
cd "$TEST_DIR"
echo "Working in temporary directory: $TEST_DIR"

# Clone the CLI repository
echo "Cloning github.com/$UPSTREAM_REPO..."
git clone "https://$GITHUB_HOST/$UPSTREAM_REPO.git"
cd testing-public-1

# Create a test branch
TEST_BRANCH="test-default-repo-$(date +%s)"
git checkout -b "$TEST_BRANCH"

# Make a small change
echo "Making test changes..."
DATE_COMMENT="// Test change made on $(date)"
echo "$DATE_COMMENT" >> README.md

git add README.md
git commit -m "test: Add date comment for testing default repo setting"

# Create PR using gh CLI
echo "Creating pull request..."
GH_DEBUG=api "$GH_BIN" pr create \
    --title "test: Verify default repo setting" \
    --body "This is a test PR to verify that the default repository is set correctly when creating PRs from forks."

# Mark that fork exists for cleanup
FORK_EXISTS=1

TEST_BRANCH="test-default-repo-$(date +%s)-2"
git checkout -b "$TEST_BRANCH"

# Make a small change
echo "Making test changes..."
DATE_COMMENT="// Test change made on $(date)"
echo "$DATE_COMMENT" >> README.md

git add README.md
git commit -m "test: Add date comment for testing default repo setting"

# Create PR using ghCLI
echo "Creating pull request..."
GH_DEBUG=api "$GH_BIN" pr create \
    --title "test: Verify default repo setting 2" \
    --body "This is a test PR 2 to verify that the default repository is set correctly when creating PRs from forks."

echo "Verifying default repository setting..."

# First list all remotes and configs for debugging
echo "Current remotes:"
git remote -v

# Now check the gh-resolved setting
DEFAULT_REPO=$(git config --get remote.upstream.gh-resolved || echo "not set")
echo "Default repository setting: $DEFAULT_REPO"

if [ "$DEFAULT_REPO" = "base" ]; then
    echo "✅ Success: Default repository was set correctly"
else
    echo "❌ Error: Default repository was not set correctly. Upstream remote exists but gh-resolved setting is missing."
    exit 1
fi

echo "Test completed successfully"

update default upstream when forking repo remove unnecessary variable

andyfeller · 2025-02-18T18:41:44Z

Thanks for opening this PR and contributing to the GitHub CLI, @daviddl9!

I'm starting to review these changes while doing a bit of manual testing to understand the new experience. Will leave my notes and review shortly.

andyfeller · 2025-02-18T18:55:23Z

Manual testing

`v2.66.1` experience

tinyfists/gh-pr-test ‹cli-10277*›$ git add .
tinyfists/gh-pr-test ‹cli-10277*›$ git commit
[cli-10277 822a8da] Add file for testing cli/cli#10277
 1 file changed, 3 insertions(+)
 create mode 100644 cli-10277.md


tinyfists/gh-pr-test ‹cli-10277›$ gh pr create
? Where should we push the 'cli-10277' branch? Create a fork of tinyfists/gh-pr-test

Creating pull request for andyfeller:cli-10277 into main in tinyfists/gh-pr-test

? Title (required) Add file for testing cli/cli#10277
? Body <Received>
? What's next? Submit
Changed tinyfists/gh-pr-test remote to "upstream"
Added andyfeller/gh-pr-test as remote "origin"
remote: Repository not found.
fatal: repository 'https://github.com/andyfeller/gh-pr-test.git/' not found
waiting 2 seconds before retrying...
remote: 
remote: 
To https://github.com/andyfeller/gh-pr-test.git
 * [new branch]      HEAD -> cli-10277
branch 'cli-10277' set up to track 'origin/cli-10277'.
https://github.com/tinyfists/gh-pr-test/pull/7


tinyfists/gh-pr-test ‹cli-10277›$ cat .git/config 
[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
	ignorecase = true
	precomposeunicode = true
[remote "upstream"]
	url = https://github.com/tinyfists/gh-pr-test.git
	fetch = +refs/heads/*:refs/remotes/upstream/*
[branch "main"]
	remote = upstream
	merge = refs/heads/main
[remote "origin"]
	url = https://github.com/andyfeller/gh-pr-test.git
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "cli-10277"]
	remote = origin
	merge = refs/heads/cli-10277

PR experience

tinyfists/gh-testing-01 ‹cli-10277*›$ git add .
tinyfists/gh-testing-01 ‹cli-10277*›$ git commit
[cli-10277 e804616] Add file to test for cli/cli#10458
 1 file changed, 3 insertions(+)
 create mode 100644 cli-10277.md


tinyfists/gh-testing-01 ‹cli-10277›$ .../bin/gh pr create
? Where should we push the 'cli-10277' branch? Create a fork of tinyfists/gh-testing-01

Creating pull request for andyfeller:cli-10277 into main in tinyfists/gh-testing-01

? Title (required) Add file to test for cli/cli#10458
? Body <Received>
? What's next? Submit
Changed tinyfists/gh-testing-01 remote to "upstream"
Added andyfeller/gh-testing-01 as remote "origin"
! Repository andyfeller/gh-testing-01 set as the default repository. To learn more about the default repository, run: gh repo set-default --help
remote: Repository not found.
fatal: repository 'https://github.com/andyfeller/gh-testing-01.git/' not found
waiting 2 seconds before retrying...
remote: 
remote: 
To https://github.com/andyfeller/gh-testing-01.git
 * [new branch]      HEAD -> cli-10277
branch 'cli-10277' set up to track 'origin/cli-10277'.
https://github.com/tinyfists/gh-testing-01/pull/1


tinyfists/gh-testing-01 ‹cli-10277›$ cat .git/config
[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
	ignorecase = true
	precomposeunicode = true
[remote "upstream"]
	url = https://github.com/tinyfists/gh-testing-01.git
	fetch = +refs/heads/*:refs/remotes/upstream/*
	gh-resolved = base
[branch "main"]
	remote = upstream
	merge = refs/heads/main
[remote "origin"]
	url = https://github.com/andyfeller/gh-testing-01.git
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "cli-10277"]
	remote = origin
	merge = refs/heads/cli-10277

jtmcg · 2025-02-18T19:31:35Z

Popping in, here: that Bash script looks like a good candidate for an acceptance test around this.

This commit brings this code more into alignment with other places: 1. Using a local variable for upstream remote name instead of hardcoding everywhere 2. Raising error raised by setting default remote 3. Removing condition for displaying upstream default message 4. Bring message logic more inline with other places within function

andyfeller

Looks good, @daviddl9! ✨

In an effort to keep the train rolling, I commit a few enhancements based on the suggestions below. Some of the other idle thoughts are things we can revisit in the future.

andyfeller · 2025-02-18T20:37:34Z

pkg/cmd/pr/create/create.go

+		if didForkRepo {
+			gitClient.SetRemoteResolution(context.Background(), "upstream", "base")
+			if opts.IO.IsStdinTTY() && opts.IO.IsStdoutTTY() {
+				cs := opts.IO.ColorScheme()
+				stderr := opts.IO.ErrOut
+				fmt.Fprintf(stderr, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(headRepo)))
+			}
+		}


Suggestions

Out of all of following idle thoughts, I think only a handful are reasonable to knock out here now versus waiting for new issues being raised:

Remove stdin requirement for messaging

Use local variable versus hardcoding remote name

Return the error raised by setting default remote

Notes

Some of my idle thoughts reviewing this logic:

Why does stdin being TTY affect whether we output this message?

How does such conditional logic apply to the Added ... as remote message?

When do we use a variable for the remote name versus hardcoding this everywhere?

How does this differ from other places in code where we handle this?

Does this logic need to ensure no other remote is marked as default before setting upstream?

TTY affecting messaging

It seems like gh repo fork care about stdin, stdout, stderr...

cli/pkg/cmd/repo/fork/fork.go

Line 187 in ae2f4b7

connectedToTerminal := opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() && opts.IO.IsStdinTTY()

cli/pkg/cmd/repo/fork/fork.go

Lines 380 to 383 in ae2f4b7

if connectedToTerminal {

fmt.Fprintf(stderr, "%s Cloned fork\n", cs.SuccessIcon())

fmt.Fprintf(stderr, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(repoToFork)))

}

whereas gh repo clone just cares about stdout being TTY:

cli/pkg/cmd/repo/clone/clone.go

Lines 217 to 221 in ae2f4b7

connectedToTerminal := opts.IO.IsStdoutTTY()

if connectedToTerminal {

cs := opts.IO.ColorScheme()

fmt.Fprintf(opts.IO.ErrOut, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(canonicalRepo.Parent)))

}

Definitely want to revisit where different types or classes of messages are displayed so we had a little more consistency. In the meantime, I assume gh pr create would follow the same behavior as gh repo clone without factoring in stdin if we even bother to check.

Setting and unsetting

My main concern with the changes as they stand is comparing this versus gh repo set-default which checks func (r Remotes) ResolvedRemote() (*Remote, error) to determine if it should unset a remote before setting a new one 🤔

cli/pkg/cmd/repo/setdefault/setdefault.go

Lines 228 to 242 in ae2f4b7

if currentDefaultRepo != nil {

if err := opts.GitClient.UnsetRemoteResolution(

ctx.Background(), currentDefaultRepo.Name); err != nil {

return err

}

}

if err = opts.GitClient.SetRemoteResolution(

ctx.Background(), selectedRemote.Name, resolution); err != nil {

return err

}

if opts.IO.IsStdoutTTY() {

cs := opts.IO.ColorScheme()

fmt.Fprintf(opts.IO.Out, "%s Set %s as the default repository for the current directory\n", cs.SuccessIcon(), ghrepo.FullName(selectedRepo))

}

It feels there is an edge case that can emerge around upstream remote separately of the server-side repository forking the command does. However, let's see about users reporting errors before dealing with that concern.

Other places where we set default repository

Just because I know this messaging is in a few different places, I always look at how consistent our behavior is.

cli/pkg/cmd/repo/clone/clone.go

Lines 209 to 221 in ae2f4b7

if err := gc.SetRemoteBranches(ctx, upstreamName, `*`); err != nil {

return err

}

if err = gc.SetRemoteResolution(ctx, upstreamName, "base"); err != nil {

return err

}

connectedToTerminal := opts.IO.IsStdoutTTY()

if connectedToTerminal {

cs := opts.IO.ColorScheme()

fmt.Fprintf(opts.IO.ErrOut, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(canonicalRepo.Parent)))

}

cli/pkg/cmd/repo/fork/fork.go

Lines 372 to 383 in ae2f4b7

if err := gc.SetRemoteResolution(ctx, upstreamRemote, "base"); err != nil {

return err

}

if err := gc.Fetch(ctx, upstreamRemote, ""); err != nil {

return err

}

if connectedToTerminal {

fmt.Fprintf(stderr, "%s Cloned fork\n", cs.SuccessIcon())

fmt.Fprintf(stderr, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(repoToFork)))

}

andyfeller · 2025-02-18T21:23:20Z

Popping in, here: that Bash script looks like a good candidate for an acceptance test around this.

@cli/code-reviewers : Perhaps its worth discussing at our next retro about when we write out acceptance tests for OSS contributions; I suspect there's a bit of tension between lowering the barrier for contributors versus ensuring user experience works as expected.

daviddl9 · 2025-02-20T05:33:20Z

Hey @andyfeller thanks for taking the time to look through the PR! Mostly agree with the ideas that you had in mind. Had a little suggestion about how we handle the case when setting the default upstream fails:

if didForkRepo {
			err := gitClient.SetRemoteResolution(context.Background(), upstreamName, "base")
			if err != nil {
				return fmt.Errorf("error setting upstream as default: %w", err)
			}

The core operation here is creating a PR. Setting the default upstream is a secondary, convenience operation that helps with future workflows but isn't critical to the PR's existence or functionality. Therefore, I'm thinking a warning would be more appropriate here since:

The PR can still function perfectly without this setting
The user can manually set the upstream later if needed
We probably shouldn't break a primary workflow due to a non-critical configuration issue

What do you think? Maybe something along the lines of:

if didForkRepo {
    if err := gitClient.SetRemoteResolution(context.Background(), upstreamName, "base"); err != nil {
        // Log warning instead of failing
        cs := opts.IO.ColorScheme()
        fmt.Fprintf(opts.IO.ErrOut, "%s Warning: Could not set upstream as default: %v\n", cs.WarningIcon(), err)
        fmt.Fprintf(opts.IO.ErrOut, "You can set it manually later using: gh repo set-default\n")
    } 
    :
}

andyfeller · 2025-02-20T15:02:24Z

The core operation here is creating a PR. Setting the default upstream is a secondary, convenience operation that helps with future workflows but isn't critical to the PR's existence or functionality. Therefore, I'm thinking a warning would be more appropriate here since:

The PR can still function perfectly without this setting

The user can manually set the upstream later if needed

We probably shouldn't break a primary workflow due to a non-critical configuration issue

What do you think? Maybe something along the lines of:
if didForkRepo {
    if err := gitClient.SetRemoteResolution(context.Background(), upstreamName, "base"); err != nil {
        // Log warning instead of failing
        cs := opts.IO.ColorScheme()
        fmt.Fprintf(opts.IO.ErrOut, "%s Warning: Could not set upstream as default: %v\n", cs.WarningIcon(), err)
        fmt.Fprintf(opts.IO.ErrOut, "You can set it manually later using: gh repo set-default\n")
    } 
    :
}

🤔 Fair concern; let's see some of the other places we're doing this:

cli/pkg/cmd/config/set/set.go

Lines 62 to 67 in 8365e0b

    
           func setRun(opts *SetOptions) error { 
        
           	err := ValidateKey(opts.Key) 
        
           	if err != nil { 
        
           		warningIcon := opts.IO.ColorScheme().WarningIcon() 
        
           		fmt.Fprintf(opts.IO.ErrOut, "%s warning: '%s' is not a known configuration key\n", warningIcon, opts.Key) 
        
           	}

cli/pkg/cmd/repo/rename/rename.go

Lines 159 to 168 in 8365e0b

    
           remote, err := updateRemote(currRepo, newRepo, opts) 
        
           if err != nil { 
        
           	if remote != nil { 
        
           		fmt.Fprintf(opts.IO.ErrOut, "%s Warning: unable to update remote %q: %v\n", cs.WarningIcon(), remote.Name, err) 
        
           	} else { 
        
           		fmt.Fprintf(opts.IO.ErrOut, "%s Warning: unable to update remote: %v\n", cs.WarningIcon(), err) 
        
           	} 
        
           } else if opts.IO.IsStdoutTTY() { 
        
           	fmt.Fprintf(opts.IO.Out, "%s Updated the %q remote\n", cs.SuccessIcon(), remote.Name) 
        
           }

@daviddl9 : what is the likelihood this will fail leaving this in a bad state?

daviddl9 · 2025-02-21T02:56:32Z

@andyfeller thanks for looking into this and for the prompt response! fair question - yeah realistically although the likelihood of failure is small, in the rare event that it does fail it could lead to a confusing / frustrating user experience where the core PR creation fails because of this. If you're ok, I could put up another small PR to change this?

andyfeller · 2025-02-21T18:09:23Z

@andyfeller thanks for looking into this and for the prompt response! fair question - yeah realistically although the likelihood of failure is small, in the rare event that it does fail it could lead to a confusing / frustrating user experience where the core PR creation fails because of this. If you're ok, I could put up another small PR to change this?

@daviddl9 : I'm going to bring this back into the issue so we can discuss it with the user who reported this as well as my fellow maintainer for discussion. It isn't clear cut whether this should be a hard or soft error; I personally think rather that this should be a hard error. Generally, we've adopted a "let's wait for this to emerge as a problem before we fix" but let's bubble this up there.

UPDATE: See #10277 (comment)

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.67.0` -> `v2.68.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.68.0`](https://github.com/cli/cli/releases/tag/v2.68.0): GitHub CLI 2.68.0 [Compare Source](cli/cli@v2.67.0...v2.68.0) #### What's Changed ##### ✨ Features - \[gh repo view] Improve error message for forked repo by [@iamazeem](https://github.com/iamazeem) in cli/cli#10334 - Add signer-digest, source-ref, and source-digest options for `gh attestation verify` by [@malancas](https://github.com/malancas) in cli/cli#10308 - \[gh pr checkout] Add --no-tags option to git fetch commands in checkout by [@latzskim](https://github.com/latzskim) in cli/cli#10479 - \[`gh issue/pr comment`] Add `--create-if-none` and prompts to create a comment if no comment already exists by [@latzskim](https://github.com/latzskim) in cli/cli#10427 - \[gh cache delete --all] Add `--succeed-on-no-caches` flag to return exit code 0 by [@iamazeem](https://github.com/iamazeem) in cli/cli#10327 - \[gh release create] Fail when there are no new commits since the last release by [@iamazeem](https://github.com/iamazeem) in cli/cli#10398 - update default upstream when forking repo during MR creation by [@daviddl9](https://github.com/daviddl9) in cli/cli#10458 ##### 🐛 Fixes - Refactor `GetLocalAttestations` and clean up custom registry transport by [@malancas](https://github.com/malancas) in cli/cli#10382 - Check `GH_REPO` too in addition to `--repo` for disambiguation by [@williammartin](https://github.com/williammartin) in cli/cli#10539 - (Fixes `gh secret` subcommands not working outside of a repository) - Fix unhandled panic in FindWorkflow and add tests by [@jtmcg](https://github.com/jtmcg) in cli/cli#10521 - Fix checkout when URL arg is from fork and cwd is upstream by [@williammartin](https://github.com/williammartin) in cli/cli#10512 - \[gh api] Escape package name (URL encoding) for packages endpoint by [@iamazeem](https://github.com/iamazeem) in cli/cli#10384 - Fix `remoteResolver` caching issue by [@iamazeem](https://github.com/iamazeem) in cli/cli#10456 - Fix gh project item-edit to allow --number 0 as a valid value by [@aryanbhosale](https://github.com/aryanbhosale) in cli/cli#10417 - Add mutex to fix race in attestation test client by [@codysoyland](https://github.com/codysoyland) in cli/cli#10439 - Base64 decode GPG passphrase in deployment workflow by [@BagToad](https://github.com/BagToad) in cli/cli#10546 ##### 📚 Docs & Chores - Deep Dive Document Release Process by [@williammartin](https://github.com/williammartin) in cli/cli#10503 - Inconsistent format of examples in help text by [@iamazeem](https://github.com/iamazeem) in cli/cli#10508 - Inconsistent format of description of flags (starting with lowercase letter) by [@iamazeem](https://github.com/iamazeem) in cli/cli#10507 - Update Go version to 1.23 in CONTRIBUTING.md by [@williammartin](https://github.com/williammartin) in cli/cli#10504 - Fix minor auth login help typo by [@williammartin](https://github.com/williammartin) in cli/cli#10501 - docs: document how to revoke `gh` OAuth tokens in `auth logout`'s help by [@BagToad](https://github.com/BagToad) in cli/cli#10490 - chore: update codespaces Go version by [@BagToad](https://github.com/BagToad) in cli/cli#10491 - Allow injection of TUFMetadataDir in tests by [@williammartin](https://github.com/williammartin) in cli/cli#10478 - refactor: use a more straightforward return value by [@beforetech](https://github.com/beforetech) in cli/cli#10489 - Use subtests in attestation verification integration tests by [@williammartin](https://github.com/williammartin) in cli/cli#10463 - Fix typo in README by [@iamazeem](https://github.com/iamazeem) in cli/cli#10445 - Update usage to lower-kebab-case by [@iamazeem](https://github.com/iamazeem) in cli/cli#10447 - Standardize URLs by [@iamazeem](https://github.com/iamazeem) in cli/cli#10429 - Remove trailing whitespace by [@iamazeem](https://github.com/iamazeem) in cli/cli#10430 #####

Dependencies - Bump actions/attest-build-provenance from 2.2.0 to 2.2.2 by [@dependabot](https://github.com/dependabot) in cli/cli#10518 - Bump github.com/go-jose/go-jose/v4 from 4.0.2 to 4.0.5 by [@dependabot](https://github.com/dependabot) in cli/cli#10499 - Bump github.com/spf13/pflag from 1.0.5 to 1.0.6 by [@dependabot](https://github.com/dependabot) in cli/cli#10338 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

update default upstream when forking repo

5004ba2

update default upstream when forking repo remove unnecessary variable

daviddl9 force-pushed the update-pr-create branch from 0605884 to 5004ba2 Compare February 17, 2025 03:14

daviddl9 changed the title ~~update default upstream when forking repo~~ update default upstream when forking repo during PR creation Feb 17, 2025

daviddl9 marked this pull request as ready for review February 17, 2025 15:30

daviddl9 requested a review from a team as a code owner February 17, 2025 15:30

daviddl9 requested a review from andyfeller February 17, 2025 15:30

cliAutomation added the external pull request originating outside of the CLI core team label Feb 17, 2025

Merge branch 'trunk' into update-pr-create

6368d53

andyfeller added a commit to andyfeller/gh-testing-01 that referenced this pull request Feb 18, 2025

Add file to test for cli/cli#10458

e804616

andyfeller approved these changes Feb 18, 2025

View reviewed changes

andyfeller merged commit c69e88e into cli:trunk Feb 18, 2025
9 checks passed

andyfeller mentioned this pull request Feb 20, 2025

gh api response should escape control characters in response ( especially diff_hunk) with respect to PR comments #10474

Closed

andyfeller mentioned this pull request Feb 21, 2025

Set default repo when creating fork during pr creation #10277

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update default upstream when forking repo during PR creation#10458

update default upstream when forking repo during PR creation#10458
andyfeller merged 3 commits intocli:trunkfrom
daviddl9:update-pr-create

daviddl9 commented Feb 17, 2025 •

edited

Loading

Uh oh!

andyfeller commented Feb 18, 2025

Uh oh!

andyfeller commented Feb 18, 2025

Uh oh!

jtmcg commented Feb 18, 2025

Uh oh!

andyfeller left a comment

Uh oh!

andyfeller Feb 18, 2025 •

edited

Loading

Uh oh!

andyfeller commented Feb 18, 2025

Uh oh!

Uh oh!

daviddl9 commented Feb 20, 2025 •

edited

Loading

Uh oh!

andyfeller commented Feb 20, 2025

Uh oh!

daviddl9 commented Feb 21, 2025

Uh oh!

andyfeller commented Feb 21, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	if connectedToTerminal {
	fmt.Fprintf(stderr, "%s Cloned fork\n", cs.SuccessIcon())
	fmt.Fprintf(stderr, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(repoToFork)))
	}

	connectedToTerminal := opts.IO.IsStdoutTTY()
	if connectedToTerminal {
	cs := opts.IO.ColorScheme()
	fmt.Fprintf(opts.IO.ErrOut, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(canonicalRepo.Parent)))
	}

	if currentDefaultRepo != nil {
	if err := opts.GitClient.UnsetRemoteResolution(
	ctx.Background(), currentDefaultRepo.Name); err != nil {
	return err
	}
	}
	if err = opts.GitClient.SetRemoteResolution(
	ctx.Background(), selectedRemote.Name, resolution); err != nil {
	return err
	}

	if opts.IO.IsStdoutTTY() {
	cs := opts.IO.ColorScheme()
	fmt.Fprintf(opts.IO.Out, "%s Set %s as the default repository for the current directory\n", cs.SuccessIcon(), ghrepo.FullName(selectedRepo))
	}

	if err := gc.SetRemoteBranches(ctx, upstreamName, `*`); err != nil {
	return err
	}

	if err = gc.SetRemoteResolution(ctx, upstreamName, "base"); err != nil {
	return err
	}

	connectedToTerminal := opts.IO.IsStdoutTTY()
	if connectedToTerminal {
	cs := opts.IO.ColorScheme()
	fmt.Fprintf(opts.IO.ErrOut, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(canonicalRepo.Parent)))
	}

	if err := gc.SetRemoteResolution(ctx, upstreamRemote, "base"); err != nil {
	return err
	}

	if err := gc.Fetch(ctx, upstreamRemote, ""); err != nil {
	return err
	}

	if connectedToTerminal {
	fmt.Fprintf(stderr, "%s Cloned fork\n", cs.SuccessIcon())
	fmt.Fprintf(stderr, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(repoToFork)))
	}

Conversation

daviddl9 commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andyfeller commented Feb 18, 2025

Uh oh!

andyfeller commented Feb 18, 2025

Manual testing

v2.66.1 experience

PR experience

Uh oh!

jtmcg commented Feb 18, 2025

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

andyfeller Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Suggestions

Notes

TTY affecting messaging

Setting and unsetting

Other places where we set default repository

Uh oh!

andyfeller commented Feb 18, 2025

Uh oh!

Uh oh!

daviddl9 commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andyfeller commented Feb 20, 2025

Uh oh!

daviddl9 commented Feb 21, 2025

Uh oh!

andyfeller commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

daviddl9 commented Feb 17, 2025 •

edited

Loading

`v2.66.1` experience

andyfeller Feb 18, 2025 •

edited

Loading

daviddl9 commented Feb 20, 2025 •

edited

Loading

andyfeller commented Feb 21, 2025 •

edited

Loading