On Fri, Feb 2, 2018 at 6:47 PM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Wed, Jan 24, 2018 at 4:53 AM, Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> wrote:
>> This command allows to delete a worktree. Like 'move' you cannot
>> remove the main worktree, or one with submodules inside [1].
>> [...]
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
>> ---
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> @@ -688,6 +689,132 @@ static int move_worktree(int ac, const char **av,
>> const char *prefix)
>> +static void check_clean_worktree(struct worktree *wt,
>> + const char *original_path)
>> +{
>> + [...]
>> + validate_no_submodules(wt);
>
> It's slightly strange seeing worktree validation in a function
> checking whether the worktree is clean since submodule validation
> isn't an issue of cleanliness. I'd have expected the caller to invoke
> it instead:
>
> int remove_worktree(...) {
> ...
> if (!force) {
> validate_no_submodules(wt);
> check_clean_worktree(wt, av[0]);
> }
> ...
> }
>
> On the other hand, I could imagine it being called here as appropriate
> if submodule validation eventually also checks submodule cleanliness
> as hinted by the commit message.
Yes I basically consider all submodules dirty until somebody sorts
them out. I'll add a comment here to clarify this.
--
Duy
