Re: [PATCH 3/5] merge-recursive: test rename threshold option

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

 



Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

> On Sun, Feb 21, 2016 at 1:55 PM, Felipe Gonçalves Assis
> <felipeg.assis@xxxxxxxxx> wrote:
>> On 21 February 2016 at 14:52, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>>> On Sun, Feb 21, 2016 at 10:09 AM, Felipe Gonçalves Assis
>>> <felipeg.assis@xxxxxxxxx> wrote:
>>>> +test_expect_success 'rename threshold is truncated' '
>>>> +       git read-tree --reset -u HEAD &&
>>>> +       test_must_fail git merge-recursive --rename-threshold=200% HEAD^ -- HEAD master &&
>>>> +       check_find_renames_100
>>>> +'
>>>
>>> Is this truncation documented behavior or is it just a detail of the
>>> current implementation. (Genuine question; I haven't checked the
>>> documentation or source.) If just an implementation detail, then it
>>> might not be desirable to formalize it via a test.
>>
>> Not documented. I will remove this. If you prefer to have it
>> documented and the test added back later, I can do that.
>
> Looking at the code itself and its history, this seems to be a
> deliberate decision, so the test may be appropriate, however, I defer
> to Junio's judgment.

This is just us not caring enough about illogical input by crazy
users who ignore the fact that the rename scores are supposed to be
between 0 and max.

We could have added code to reject out-of-range values when we added
support for input with '%', but we instead just made sure we clip
the result within the range (probably because it was easier).

I personally do not think it would break too many people if we
changed the parser to error out, but at the same time, I do not
think it is worth the code churn to do so, and it probably is not
worth to document the clipping either.


[Footnote]

*1* With the original "give us an integer, and we'll take it as a
ratio of that number out of the next power of ten", there was no
need to worry about out-of-range values.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]