[PATCH V2] rebase: do not raise an UnboundLocalError when called with wrong rev (issue4106)
simohe at besonet.ch
Thu Feb 20 03:05:19 CST 2014
I will send V3 soon. More details below.
On 2014-02-19 20:55, David Soria Parra wrote:
> On 2/19/14, 2:55 AM, "Simon Heimberg" <simohe at besonet.ch> wrote:
>> diff -r 0e2877f8605d -r 2554f90793b6 hgext/rebase.py
>> --- a/hgext/rebase.py Sat Feb 15 22:09:32 2014 -0600
>> +++ b/hgext/rebase.py Fri Feb 14 00:34:20 2014 +0100
>> @@ -513,6 +513,11 @@
>> if state.get(p.rev()) == repo[p1].rev():
>> base = p.node()
>> + else:
>> + # fallback when base not found (unnormal, see issue 4106)
>> + #
>> + # Raise because this function is (probaby) called wrong
>> + raise AssertionError('function is called wrong')
> 1. I think we could do better as Œfunction is called wrong¹. Maybe Œbase
> must be defined¹.
We can not mention the variable "base", the caller can not define it.
Maybe "no base found to rebase on (rebasenode called wrong)"?
> 2. I personally would have an invariant before the
> assert Œbase¹ in locals()
> if base is not None:
> making it clear that base needs to be defined after the whole if-else
We can add "base = None" in else, it is always undefined there. Then we
could just run on, the problem of not finding a valid base would be
reported later (somehow).
By raising an AssertionError, we make clear that the caller of this
function did something wrong. (And raising AssertionError is never
optimized away, unlike assert.) The patch description tells this case
only happens when an extension calls rebasenode() wrong.
> - David
More information about the Mercurial-devel