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

[Martin von Zweigbergk]

On Wed, Jun 14, 2017 at 12:04 PM, Augie Fackler <raf at durin42.com> wrote:
>
>> 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)

Ah, there it is. Thanks. I did look, but I couldn't find it.

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

Yeah, seems safe since it's explicitly converted to int. If we do end
up backing it out, we should also update bookmarks.py because that
already use False/True (that's how I learned that those were valid
values).

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