Re: ds/sparse-checkout-requires-per-worktree-config (was Re: What's cooking in git.git (Feb 2022, #02; Wed, 9))

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 11, 2022, 16:51 UTC
Message-ID: <xmqqfsope48k.fsf@gitster.g>
In-Reply-To: <f36712f5-28bd-42d7-3ea1-f4afa328be07@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 13 quoted lines

> On 2/9/2022 7:12 PM, Junio C Hamano wrote:
>
>> * ds/sparse-checkout-requires-per-worktree-config (2022-02-08) 6 commits
>> ...
>>  Will merge to 'next'?
>>  cf. <20220204081336.3194538-1-newren@gmail.com>
>>  cf. <CAPig+cRrRxuTeByhKkLs_KDaWY8-r4+jrwT83A-r+sBQsmebMw@mail.gmail.com>
>>  source: <pull.1101.v6.git.1644269583.gitgitgadget@gmail.com>
>
> You and I have had a good discussion about the latest version. I
> think we've mostly landed on finding ways to improve documentation
> in other ways (including the patch you submitted), but here are
> the things that I see as still outstanding:

Thanks for a clearly written summary.  Very much appreciated.

With the maintainer hat on, I agree that all are good points. From a reviewer's point of view, I do not care too deeply about 1 or 2 myself though.

Show 38 quoted lines

> 0. Eric mentioned earlier that he was interested in looking again,
>    but has not signaled that his review is complete.
>
> 1. You and Eric disagree about the use of "worktree" and "working
>    tree" in the documentation. I could revert the change to these
>    docs from v5 to v6.
>
> 2. You mention that the changes in config.c could be split into
>    two (first, create repo_config_set_multivar_gently() and then
>    create repo_config_set_worktree_gently()).
>
> 3. Jean-Noël noticed an improvement to reduce work on translators.
>    The diff below could be squashed into patch 5 OR I could submit
>    it as a forward fix.
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index c6eb636329a..7c272078dc9 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -384,11 +384,13 @@ static int add_worktree(const char *path, const char *refname,
>  			    bare &&
>  			    git_config_set_multivar_in_file_gently(
>  					to_file, "core.bare", NULL, "true", 0))
> -				error(_("failed to unset 'core.bare' in '%s'"), to_file);
> +				error(_("failed to unset '%s' in '%s'"),
> +				      "core.bare", to_file);
>  			if (!git_configset_get_value(&cs, "core.worktree", &core_worktree) &&
>  			    git_config_set_in_file_gently(to_file,
>  							  "core.worktree", NULL))
> -				error(_("failed to unset 'core.worktree' in '%s'"), to_file);
> +				error(_("failed to unset '%s' in '%s'"),
> +				      "core.worktree", to_file);
>  
>  			git_configset_clear(&cs);
>  		}
>
> Thanks,
> -Stolee