From: Ben Peart <peartben@gmail.com>
Date: Wed, 02 May 2018 14:33:09 GMT
Subject: Re:
Message-ID: <c5c262f7-c24e-46bf-e9c8-24b322543711@gmail.com>
In-Reply-To: <CABPp-BEC2cnpdvDsMPFodvNR06G5E434Hpdmaex+6+zHpYm_QQ@mail.gmail.com>



On 4/30/2018 12:12 PM, Elijah Newren wrote:
> On Mon, Apr 30, 2018 at 6:11 AM, Ben Peart <peartben@gmail.com> wrote:
>> On 4/27/2018 2:19 PM, Elijah Newren wrote:
>>>
>>> From: Elijah Newren <newren@gmail.com>
>>>
>>> On Thu, Apr 26, 2018 at 5:54 PM, Ben Peart <peartben@gmail.com> wrote:
>>>
>>>> Can you write the documentation that clearly explains the exact behavior
>>>> you
>>>> want?  That would kill two birds with one stone... :)
>>>
>>>
>>> Sure, something like the following is what I envision, and I've tried to
>>> include the suggestion from Junio to document the copy behavior in the
>>> merge-recursive documentation.
>>>
> <snip>
>>
>> Thanks Elijah. I've applied this patch and reviewed and tested it.  It works
>> and addresses the concerns around the settings inheritance from
>> diff.renames.  I still _prefer_ the simpler model that doesn't do the
>> partial inheritance but I can use this model as well.
>>
>> I'm unsure on the protocol here.  Should I incorporate this patch and submit
>> a reroll or can it just be applied as is?
> 
> I suspect you'll want to re-roll anyway, to base your series on
> en/rename-directory-detection-reboot instead of on master.  (Junio
> plans to merge it down to next, and your series has four different
> merge conflicts with it.)
> 
> There are two other loose ends with this series that Junio will need
> to weigh in on:
> 
> - I'm obviously a strong proponent of the inherited setting, but Junio
> may change his mind after reading Dscho's arguments against it (or
> after reading my arguments for it).
> 
> - I like the setting as-is, and think we could allow a "copy" setting
> for merge.renames to specify that the post-merge diffstat should
> detect copies (not part of your series, but a useful addition I'd like
> to tackle afterwards).  However, Junio had comments in
> xmqqwox19ohw.fsf@gitster-ct.c.googlers.com about merge.renames
> handling the scoring as well, like -Xfind-renames.  Those sound
> incompatible to me for a single setting, and I'm unsure if Junio would
> resolve them the way I do or still feels strongly about the scoring.
> 

I think this patch series (including Elijah's fixup!) improves the 
situation from where we were and it provides the necessary functionality 
to solve the problem I started out to solve.  While there are other 
changes that could be made, I think they should be done in separate 
follow up patches.

I'm happy to reroll this incorporating the fixup! so that we can make 
progress.  Junio, would you prefer I reroll this based on 
en/rename-directory-detection-reboot or master?