[PATCH STABLE] rebase: fix rebase aborts when tip^ is public (issue4082)

Durham Goode durham at fb.com
Tue Nov 5 11:59:22 CST 2013


On 11/5/13 1:16 AM, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
wrote:

>On 5 nov. 2013, at 05:09, Durham Goode wrote:
>
>> --- a/mercurial/phases.py
>> +++ b/mercurial/phases.py
>> @@ -185,6 +185,8 @@
>>         # be replaced without us being notified.
>>         if rev == nullrev:
>>             return public
>> +        if rev < nullrev:
>> +            raise error.RepoLookupError(_('cannot lookup negative
>>revision'))
>>         if self._phaserevs is None or rev >= len(self._phaserevs):
>>             self._phaserevs = self.getphaserevs(repo, rebuild=True)
>>         return self._phaserevs[rev]
>
>I'm ok with such change, but it belong to its own patch. I'm not
>convinced RepoLookupError is right here. ValueError seems better as rev <
>-1 are out of the scope (or IndexError maybe).

What is the benefit of putting it in it's own patch? It doesn't make the
changes any more understandable (they're already tiny), and it's highly
related to the other changes being made, so someone looking back in
history would probably want to see all of them together.  I'm not a fan of
micro-patches for the sake of micro-patches and feel they clutter the
history when done unecessarily.



More information about the Mercurial-devel mailing list