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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> ...
>> diff --git a/repo-settings.c b/repo-settings.c
>> index af93696343..e0673938c0 100644
>> --- a/repo-settings.c
>> +++ b/repo-settings.c
>> @@ -34,6 +34,18 @@ void prepare_repo_settings(struct repository *r)
>>                 free(strval);
>>         }
>>
>> +       if (!repo_config_get_maybe_bool(r, "merge.directoryrenames", &value))
>> +               r->settings.merge_directory_renames = value ? MERGE_DIRECTORY_RENAMES_TRUE : 0;
>
> Shouldn't that be "MERGE_DIRECTORY_RENAMES_NONE" instead of "0"?
>
>> diff --git a/repository.h b/repository.h
>> index e7a72e2341..b8e52dd48f 100644
>> --- a/repository.h
>> +++ b/repository.h
>> @@ -19,6 +19,20 @@ enum untracked_cache_setting {
>>         UNTRACKED_CACHE_WRITE = 2
>>  };
>>
>> +enum merge_directory_renames_setting {
>> +       MERGE_DIRECTORY_RENAMES_UNSET = -1,
>> +       MERGE_DIRECTORY_RENAMES_NONE = 0,
>> +       MERGE_DIRECTORY_RENAMES_CONFLICT = 1,
>> +       MERGE_DIRECTORY_RENAMES_TRUE = 2,
>> +};
>
> Thanks for adding these; makes things much nicer.  :-)
>
>
> Cheers,
> Elijah



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux