From: Eric Sunshine Date: Tue, 27 Mar 2018 08:58:19 GMT Subject: Re: [PATCH v5 0/6] worktree: teach "add" to check out existing branches Message-ID: In-Reply-To: <20180325134947.25828-1-t.gummerer@gmail.com> On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer wrote: > Thanks Eric for the review of the previous round and Duy and Junio for > additional comments. > This round should address all of Eric's comments from the previous round. Thanks, it appears to cover my review comments from the previous round. I do have some additional comments on this round (which I could have raised with the previous round if I had thought of them at the time). > As explained in more detail in a reply to the review comment directly, > I did not add an enum to 'struct add_opts', for 'force_new_branch' and > 'checkout_existing_branch', but instead removed 'force_new_branch' > from the struct as it's not required. Makes sense. In fact, I had thoughts along these lines during your previous dwim-ery series. See my comments on patch 3/6. > The rest of the updates are mainly in the user facing messages, > documentation and one added test. > Interdiff below: The interdiff looks sane. Unfortunately, due to UI regressions, I'm having second thoughts about whether this series is going in the right direction. See my comments on patch 2/6.