Re: [PATCH v3 5/5] repo-settings: create feature.experimental setting

> On Thu, Aug 8, 2019 at 12:12 PM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> On 8/8/2019 2:59 PM, Junio C Hamano wrote:
>>> Elijah Newren <newren@gmail.com> writes:
>>>
>>>>> --- a/Documentation/config/merge.txt
>>>>> +++ b/Documentation/config/merge.txt
>>>>> @@ -54,7 +54,8 @@ merge.directoryRenames::
>>>>>         moved into the new directory.  If set to "conflict", a conflict
>>>>>         will be reported for such paths.  If merge.renames is false,
>>>>>         merge.directoryRenames is ignored and treated as false.  Defaults
>>>>> -       to "conflict".
>>>>> +       to "conflict" unless `feature.experimental` is enabled and the
>>>>> +       default is "true".
>>>>
>>>> I have a hard time parsing that changed sentence.  Perhaps something like:
>>>>    ...unless `feature.experimental` is enabled in which case the
>>>> default is "true".
>>>> ?
>>>
>>> That reads better.
>>>
>>> But I am not sure about the wisdom of controlling between conflict
>>> and true with this feature macro in the first place.
>>>
>>> Between "conflict" and "true", the former forces the end user to
>>> verify (or allows the end user to veto) the auto resolution by the
>>> heuristics and is always a safer if more cumbersome option.  It's
>>> not like blindly trusting the directory rename heuristics is the
>>> bright future for all users, is it?
>>>
>>> I would not set rerere.autoUpdate to true when feature.experimental
>>> is set; for exactly the same reason, I do not find it reasonable to
>>> set this to true with feature.experimental macro.
>>
>> OK. I can remove it from the feature.experimental variable.
>>
>> Shall I keep the enum logic and the use of repo-settings.c? I can split
>> them out into a separate patch.
> 
> Good question.  In part, I was looking at this ds/feature-macros
> series because my cleanup-merge-api series[1] has some minor conflicts
> with it.  I'm a little unsure what route I should take with my series
> now.  Some choices:
>   * keep this logic in your series in a separate patch, and have me
> rebase my series on yours.
>   * drop this logic from your series since it may not be needed
> anymore, making our two series independent again.
>   * move this logic into a separate patch of yours but making that
> patch part of my series instead; that'd be easy with the enum logic,
> but the repo-settings.c stuff appears to depend on your other
> patches...
> 
> Thoughts or preferences?