Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add --single-branch flag for dolt clone #7088

Merged
merged 6 commits into from
Dec 6, 2023
Merged

add --single-branch flag for dolt clone #7088

merged 6 commits into from
Dec 6, 2023

Conversation

stephkyou
Copy link
Contributor

Adds --single-branch flag for dolt clone to match git behavior more closely.

Fixes: #3873

@stephkyou stephkyou marked this pull request as ready for review December 1, 2023 23:48
go/cmd/dolt/cli/arg_parser_helpers.go Outdated Show resolved Hide resolved
integration-tests/bats/remotes.bats Show resolved Hide resolved
go/libraries/doltcore/env/actions/clone.go Outdated Show resolved Hide resolved
if brnch.GetPath() != branch {
err := dEnv.DoltDB.DeleteBranch(ctx, brnch, nil)
if err != nil {
return fmt.Errorf("%w: %s; %s", ErrFailedToDeleteBranch, brnch.String(), err.Error())
}
} else if !singleBranch || brnch.GetPath() == branch {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only way you can reach this conditional is if the branches are equal (because we checked for inequality in the first conditional), so this block will always be entered regardless of the singleBranch flag, because anything || TRUE is always true.

You can convince me this is right if you can point me to tests which verify the remote tracking branches work without the --single-branch (or write a test for that if we don't have one).

Copy link
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ship when the nit we talked about is resolved and the tests continue to pass.

if err != nil {
return fmt.Errorf("%w: %s; %s", ErrFailedToResolveBranchRef, brnch.String(), err.Error())
for _, br := range branches {
if !singleBranch || (singleBranch && br.GetPath() == branch) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small logical redundancy we talked about.

@stephkyou stephkyou merged commit ed7ee23 into main Dec 6, 2023
17 checks passed
@stephkyou stephkyou deleted the steph/clone-branch branch December 6, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dolt clone -branch should omit remote tracking branches for other branches
2 participants