[PATCH] pushkey: use False/True for return values from push functions

Augie Fackler raf at durin42.com
Wed Jun 14 15:04:19 EDT 2017


> On Jun 14, 2017, at 12:07, Martin von Zweigbergk via Mercurial-devel <mercurial-devel at mercurial-scm.org> wrote:
> 
> On Wed, Jun 14, 2017 at 5:52 AM, Yuya Nishihara <yuya at tcha.org> wrote:
>> On Wed, 14 Jun 2017 13:15:22 +0200, Pierre-Yves David wrote:
>>> On 06/13/2017 08:54 PM, Sean Farley wrote:
>>>> Martin von Zweigbergk via Mercurial-devel
>>>> <mercurial-devel at mercurial-scm.org> writes:
>>>> 
>>>>> # HG changeset patch
>>>>> # User Martin von Zweigbergk <martinvonz at google.com>
>>>>> # Date 1497310557 25200
>>>>> #      Mon Jun 12 16:35:57 2017 -0700
>>>>> # Node ID 984cdd0844fecb6c56d570236b03c999c4d485cf
>>>>> # Parent  f40eec7af04416521543b284fc6fa5365dbef611
>>>>> pushkey: use False/True for return values from push functions
>>>>> 
>>>>> It was particularly unclear in phases.pushphase() whether the 0/1
>>>>> returned were the 0/1 for public/draft phase or for False/True
>>>> 
>>>> Looks good to me; queued!
>>> 
>>> I'm not sure this is a good idea, the pushkey return treated as an
>>> integer everywhere in the code, including the bit handling phases. Using
>>> "True/False" works because they are also integers but that does not feel
>>> like a good idea to me.
>> 
>> Agreed. It seems these return values derive from the wireproto layer.
> 
> For both of you: Where specifically?

from wireproto.py:

@wireprotocommand('pushkey', 'namespace key old new')
def pushkey(repo, proto, namespace, key, old, new):
[...]
    r = repo.pushkey(encoding.tolocal(namespace), encoding.tolocal(key),
                     encoding.tolocal(old), new)
    return '%s\n' % int(r)

But I think this patch is fine. I'm going to mark it as accepted, and if we end up worried about this we can back it out. :)

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel



More information about the Mercurial-devel mailing list