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:
Show 38 quoted lines
>
> 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?
[1] https://public-inbox.org/git/20190726155258.28561-1-newren@gmail.com/
Elijah Newren <newren@gmail.com>