From: Duy Nguyen <pclouds@gmail.com>
Date: Mon, 12 Feb 2018 09:26:16 GMT
Subject: Re: [PATCH 6/7] worktree remove: new command
Message-ID: <CACsJy8BsdkZFWE2vaWtobMMum85U5vu-iu9MctaWn6vzqsZ3Vg@mail.gmail.com>
In-Reply-To: <CAPig+cQvVreeggOj52bVLS6R0ZCar_yfUcyLbN3-nLG81eOQaA@mail.gmail.com>

On Fri, Feb 2, 2018 at 6:47 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Jan 24, 2018 at 4:53 AM, Nguyễn Thái Ngọc Duy <pclouds@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 <pclouds@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