[PATCH 3 of 7 V2] transaction: add onclose/onabort hook for pre-close logic

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Apr 1 00:26:28 UTC 2014



On 03/31/2014 05:24 PM, Gregory Szorc wrote:
> On 3/31/14, 5:16 PM, Pierre-Yves David wrote:
>>
>>
>> On 03/31/2014 05:14 PM, Gregory Szorc wrote:
>>> On 3/31/14, 5:07 PM, Pierre-Yves David wrote:
>>>>
>>>>
>>>> On 03/31/2014 04:19 PM, Durham Goode wrote:
>>>>> +        if self.count == 1 and self.onclose:
>>>>> +            self.onclose()
>>>>
>>>> `onclose is not None`
>>>>
>>>> Please, otherwise this is going to bit someone in 31.4 months
>>>>
>>>>> @@ -149,6 +155,9 @@
>>>>>           self.usages = 0
>>>>>           self.file.close()
>>>>>
>>>>> +        if self.onabort:
>>>>> +            self.onabort()
>>>>
>>>> `onabort is not None`
>>>
>>> This is an anti-pattern in Python. Unless you are trying to
>>> differentiate None from False from [] from {} from '' from any other
>>> type whose __nonzero__ can return False, the explicit type comparison
>>> can be safely left out.
>>
>>
>> For me, the contrary is the anti pattern. it is very easy to slip
>> variable type to something that may exist and still be false.
>>
>> I've lost multiple days in the last 5 years because of people not using
>> `is not None`
>>
>> (explicit is better than implicit)
>
> I don't see how that applies here as you are calling self.onclose and
> self.onabort immediately after the check. That will raise if the thing
> isn't callable.

until the code drift appart or some sort of strange evaluated to false 
but still callable object is added.

(special case are not special enough)

-- 
Pierre-Yves


More information about the Mercurial-devel mailing list