Set upstream remote to track all branches after initial fetch #7542

samcoe · 2023-06-07T07:34:10Z

After running repo clone set the upstream remote to track all branches instead of just the default branch.

This PR also adds a Copy() method to the git client as that pattern had been used a couple of times across the codebase.

Fixes #5385

samcoe · 2023-06-07T07:34:50Z

git/client.go

-func (c *Client) IsLocalGitRepo(ctx context.Context) (bool, error) {
-	_, err := c.GitDir(ctx)
-	if err != nil {
-		var execError errWithExitCode
-		if errors.As(err, &execError) && execError.ExitCode() == 128 {
-			return false, nil
-		}
-		return false, err
-	}
-	return true, nil
-}
-
-func (c *Client) UnsetRemoteResolution(ctx context.Context, name string) error {
-	args := []string{"config", "--unset", fmt.Sprintf("remote.%s.gh-resolved", name)}
-	cmd, err := c.Command(ctx, args...)
-	if err != nil {
-		return err
-	}
-	_, err = cmd.Output()
-	if err != nil {
-		return err
-	}
-	return nil
-}


These just got moved upwards in the file, no actual changes to the methods.

williammartin

Makes sense to me! Thanks.

williammartin · 2023-06-08T13:23:08Z

pkg/cmd/repo/clone/clone.go

+		gc := gitClient.Copy()
+		gc.RepoDir = cloneDir
+
+		_, err = gc.AddRemote(ctx, upstreamName, upstreamURL, []string{canonicalRepo.Parent.DefaultBranchRef.Name})


I don't know about you but I kind of like the WithRepoDir modifier pattern, it's just a shame that it doesn't "put it back the way it was" so to speak.

I am not exactly following. The WithRepoDir modifier does keep the git client directory unchanged and just modifies the single command, but in this case we were now up to 3 commands that needed to be modified so it seemed like an appropriate time to create a new git client specifically targeted at the cloneDir directory.

My mistake! I thought it modified the RepoDir field on the git client rather than appending a -C just for this command.

I actually quite like that pattern even if used multiple times. Generally, I find copy semantics in languages with limited mutation control to be difficult to reason about and a regular source of "oops I modified this thing here and I didn't expect it to be modified there".

Not a worry for this though, I think it's a sensible pattern to explore. Thanks for the correction.

Set upstream remote to track all branches after initial fetch

49ac1d4

samcoe requested a review from a team as a code owner June 7, 2023 07:34

samcoe requested review from vilmibm and removed request for a team June 7, 2023 07:34

samcoe self-assigned this Jun 7, 2023

samcoe commented Jun 7, 2023

View reviewed changes

williammartin approved these changes Jun 8, 2023

View reviewed changes

samcoe merged commit fac4971 into trunk Jun 12, 2023

samcoe deleted the fix-repo-clone-fetch branch June 12, 2023 00:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set upstream remote to track all branches after initial fetch #7542

Set upstream remote to track all branches after initial fetch #7542

samcoe commented Jun 7, 2023

samcoe Jun 7, 2023

williammartin left a comment

williammartin Jun 8, 2023

samcoe Jun 12, 2023

williammartin Jun 12, 2023

Set upstream remote to track all branches after initial fetch #7542

Set upstream remote to track all branches after initial fetch #7542

Conversation

samcoe commented Jun 7, 2023

samcoe Jun 7, 2023

Choose a reason for hiding this comment

williammartin left a comment

Choose a reason for hiding this comment

williammartin Jun 8, 2023

Choose a reason for hiding this comment

samcoe Jun 12, 2023

Choose a reason for hiding this comment

williammartin Jun 12, 2023

Choose a reason for hiding this comment