On Tue, Mar 29, 2016 at 6:11 AM, Ray Zhang <zhanglei002@xxxxxxxxx> wrote:
> By adding this option which defaults to true, we can use the
> corresponding --no-checkout to make some customizations before
> the checkout, like sparse checkout, etc.
>
> Reviewed-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> Signed-off-by: Ray Zhang <zhanglei002@xxxxxxxxx>
> ---
> Changes since last version of this patch[v4]:
> t/t2025-worktree-add.sh: use test -e to test file existence.
> builtin/worktree.c: refactor the code a little bit.
Thanks, this version is still:
Reviewed-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
A couple comments below...
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -284,18 +285,22 @@ static int add_worktree(const char *path, const char *refname,
> if (ret)
> goto done;
>
> - cp.argv = NULL;
> - argv_array_clear(&cp.args);
> - argv_array_pushl(&cp.args, "reset", "--hard", NULL);
> - cp.env = child_env.argv;
> - ret = run_command(&cp);
> - if (!ret) {
> - is_junk = 0;
> - free(junk_work_tree);
> - free(junk_git_dir);
> - junk_work_tree = NULL;
> - junk_git_dir = NULL;
> + if (opts->checkout) {
> + cp.argv = NULL;
> + argv_array_clear(&cp.args);
> + argv_array_pushl(&cp.args, "reset", "--hard", NULL);
> + cp.env = child_env.argv;
> + ret = run_command(&cp);
> + if (ret)
> + goto done;
> }
> +
> + is_junk = 0;
> + free(junk_work_tree);
> + free(junk_git_dir);
> + junk_work_tree = NULL;
> + junk_git_dir = NULL;
Doing the goto-dance and outdenting the "freeing" code as suggested as
a possible improvement by [1] probably should have been done as a
separate preparatory patch since the result in this patch is fairly
noisy and more difficult to review. However, it's probably not worth
the patch churn to do so now.
> done:
> strbuf_reset(&sb);
> strbuf_addf(&sb, "%s/locked", sb_repo.buf);
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> +test_expect_success '"add" worktree with --no-checkout' '
> + git worktree add --no-checkout -b swamp swamp &&
> + ! test -e swamp/init.t &&
I realize that this was suggested by [2], however, a more modern way
to state this would be:
test_path_is_missing swamp/init.t &&
but, as also mentioned in [2], it's probably not worth the patch churn
to change it now.
> + git -C swamp reset --hard &&
> + test_cmp init.t swamp/init.t
> +'
> +
> +test_expect_success '"add" worktree with --checkout' '
> + git worktree add --checkout -b swmap2 swamp2 &&
> + test_cmp init.t swamp2/init.t
> +'
> +
> test_done
[1]: http://git.661346.n2.nabble.com/PATCH-add-option-n-no-checkout-to-git-worktree-add-tp7651385p7651884.html
[2]: http://article.gmane.org/gmane.comp.version-control.git/290050
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html