On 8/8/2019 4:31 PM, Elijah Newren wrote: > On Thu, Aug 8, 2019 at 12:12 PM Derrick Stolee <stolee@xxxxxxxxx> wrote: >> >> On 8/8/2019 2:59 PM, Junio C Hamano wrote: >>> Elijah Newren <newren@xxxxxxxxx> 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? I've done the work in my GGG PR [1] to split the commit into two, with the merge.directoryRenames on top. I created a tag [2] at that commit and I'll send my v4 without changes to merge.directoryRenames. I'll let you decide if you like placing the config in repo-settings or not (or want to adapt the enum logic in a different place). Thanks, -Stolee [1] https://github.com/gitgitgadget/git/pull/292 [2] https://github.com/derrickstolee/git/releases/tag/merge.directoryRenames
