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

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Dec 2 14:35:19 CST 2014



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, I dunno when 
I'll have time to review your V2.

> 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.

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list