Re: [PATCH v5 0/6] worktree: teach "add" to check out existing branches
- From
Thomas Gummerer <t.gummerer@gmail.com>
- Date
- Mar 30, 2018, 14:08 UTC
- Message-ID
- <20180330140839.GD2629@hank>
- In-Reply-To
- <CAPig+cSS2T-CweGtfO1vm=cskS91hLCHBoOhDuoqHAzg=27WbA@mail.gmail.com>
On 03/27, Eric Sunshine wrote:
Show 25 quoted lines
> On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer <t.gummerer@gmail.com> 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.
Thanks for your reviews of this series! As I mentioned in the reply there I'm going to see whether or not I can fix those regressions (hopefully I can :)), and send a re-roll.