[PATCH 06 of 17] rebase: use nullrev instead of -1

Mads Kiilerich mads at kiilerich.com
Tue Dec 2 19:17:18 CST 2014


On 12/02/2014 09:35 PM, Pierre-Yves David wrote:
>
>
> On 12/01/2014 08:24 PM, Mads Kiilerich wrote:
>> On 12/01/2014 02:36 PM, Pierre-Yves David wrote:
>>> On 12/01/2014 05:24 AM, Mads Kiilerich wrote:
>>>> On 12/01/2014 08:13 AM, Pierre-Yves David wrote:
>>>>> On 11/30/2014 11:08 AM, Mads Kiilerich wrote:
>>>>>> # HG changeset patch
>>>>>> # User Mads Kiilerich <madski at unity3d.com>
>>>>>> # Date 1417374421 -3600
>>>>>> #      Sun Nov 30 20:07:01 2014 +0100
>>>>>> # Node ID 09fe6c1db24cc0c3fd3dceae4063c0a8dcbd11a5
>>>>>> # Parent  0cff65a0d024e31284ff6eb804ed4ac65f628a6e
>>>>>> rebase: use nullrev instead of -1
>>>>>
>>>>> irk!
>>>>>
>>>>> Using constant is good. Using unrelated constant because they happen
>>>>> to have the same value is wrong.
>>>>
>>>> The code already use the nullrev constant:
>>>> http://selenic.com/hg/file/b913c394386f/hgext/rebase.py#l783
>>>> http://selenic.com/hg/file/b913c394386f/hgext/rebase.py#l810
>>>> http://selenic.com/hg/file/b913c394386f/hgext/rebase.py#l875
>>>>
>>>> I do not disagree that it probably would be better to use a different
>>>> constant (and preferably a different value). The scope if this 
>>>> change is
>>>> however just consistent use of the existing, not to introduce 
>>>> something
>>>> new.
>>>
>>> -1 for moving in the wrong direction. Lets fix the constant name
>>> instead. The fix is super simple lets do it while it is in our mind.
>>
>> If you think it is super simple then go ahead.
>
> Sure, now that I've been spending my time doing your work

It was you who wanted to redefine the scope to use another constant than 
nullrev, not me.

>
>> Make sure you test that the new constant really is independent of
>> nullrev and doesn't have to be -1.
>
> Sure, this actually caught a bug (by pure luck) in the process.

I wouldn't call it luck. The debugging of this "something" was outside 
the scope of the cleanup I was trying to do and did not match my 
definition of "super simple".

/Mads


More information about the Mercurial-devel mailing list