[PATCH 2 of 2] patch.internalpatch: accept a prefix parameter

Pierre-Yves David pierre-yves.david at ens-lyon.org
Mon Mar 9 21:55:17 CDT 2015



On 03/09/2015 11:28 AM, Siddharth Agarwal wrote:
> On 03/09/2015 11:25 AM, Pierre-Yves David wrote:
>> On 03/09/2015 09:59 AM, Siddharth Agarwal wrote:> # HG changeset patch
>>> # User Siddharth Agarwal <sid0 at fb.com>
>>> # Date 1425710627 28800
>>> #      Fri Mar 06 22:43:47 2015 -0800
>>> # Node ID ee9d52883035d7d32b18421b33653e597f4e4cf5
>>> # Parent  f4f59f9484d44f3941b13b76d168f50e91c3cbb3
>>> patch.internalpatch: accept a prefix parameter
>>>
>>
>> These patches looks good to me
>> (although this would have been a great occasion to add some
>> documentation to these functions)
>>
>> I'm not sure why you do not provide a default value for the argument.
>> But this probably have to do with breaking the signature (but would be
>> happy to have clarification).
>
> (1) In general I don't like default arguments, especially for internal
> methods.
> (2) prefix and strip logically go together, so wherever strip doesn't
> have a default argument I didn't have one for prefix either.

Ok, as nobody growled about them, they are pushed to the clowncopter.


-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list