fix race condition, increase clarity

nate smith · nate smith · commit 667671f04810 · 2022-01-25T15:36:41.000-06:00
diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go
@@ -182,14 +182,6 @@ func forkRun(opts *ForkOptions) error {
 		return fmt.Errorf("failed to fork: %w", err)
 	}
 
-	// Rename the forked repo if ForkName is specified in opts.
-	if opts.ForkName != "" {
-		forkedRepo, err = api.RenameRepo(apiClient, forkedRepo, opts.ForkName)
-		if err != nil {
-			return err
-		}
-	}
-
 	// This is weird. There is not an efficient way to determine via the GitHub API whether or not a
 	// given user has forked a given repo. We noticed, also, that the create fork API endpoint just
 	// returns the fork repo data even if it already exists -- with no change in status code or
@@ -211,6 +203,17 @@ func forkRun(opts *ForkOptions) error {
 		}
 	}
 
+	// Rename the forked repo if ForkName is specified in opts.
+	if opts.ForkName != "" {
+		forkedRepo, err = api.RenameRepo(apiClient, forkedRepo, opts.ForkName)
+		if err != nil {
+			return fmt.Errorf("could not rename fork: %w", err)
+		}
+		if connectedToTerminal {
+			fmt.Fprintf(stderr, "%s Renamed fork to %s\n", cs.SuccessIconWithColor(cs.Green), cs.Bold(ghrepo.FullName(forkedRepo)))
+		}
+	}
+
 	if (inParent && (!opts.Remote && !opts.PromptRemote)) || (!inParent && (!opts.Clone && !opts.PromptClone)) {
 		return nil
 	}
diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go
@@ -614,7 +614,7 @@ func TestRepoFork(t *testing.T) {
 					httpmock.REST("PATCH", "repos/OWNER/REPO"),
 					httpmock.StringResponse(renameResult))
 			},
-			wantErrOut: "✓ Created fork OWNER/NEW_REPO\n",
+			wantErrOut: "✓ Created fork OWNER/REPO\n✓ Renamed fork to OWNER/NEW_REPO\n",
 		},
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -614,7 +614,7 @@ func TestRepoFork(t *testing.T) {`
`614`	`614`	`httpmock.REST("PATCH", "repos/OWNER/REPO"),`
`615`	`615`	`httpmock.StringResponse(renameResult))`
`616`	`616`	`},`
`617`		`- wantErrOut: "✓ Created fork OWNER/NEW_REPO\n",`
	`617`	`+ wantErrOut: "✓ Created fork OWNER/REPO\n✓ Renamed fork to OWNER/NEW_REPO\n",`
`618`	`618`	`},`
`619`	`619`	`}`
`620`	`620`