[PATCH 2 of 2] exchange: refactor push so that extensions can wrap pushop

[Ryan McElroy]

On 10/15/2015 10:45 AM, Sean Farley wrote:
> Pierre-Yves David <pierre-yves.david at ens-lyon.org> writes:
>
>> On 10/15/2015 06:13 PM, Gregory Szorc wrote:
>>> On Thu, Oct 15, 2015 at 3:28 AM, Pierre-Yves David
>>> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
>>> wrote:
>>>
>>>
>>>
>>>      On 10/15/2015 07:03 AM, Sean Farley wrote:
>>>
>>>          # HG changeset patch
>>>          # User Sean Farley <sean at farley.io <mailto:sean at farley.io>>
>>>          # Date 1444802693 25200
>>>          #      Tue Oct 13 23:04:53 2015 -0700
>>>          # Node ID 9984305b7184ceea37cb266f64f40e1021bdc10d
>>>          # Parent  eee993036c7c1593dbef90ae639cca5c07f10594
>>>          exchange: refactor push so that extensions can wrap pushop
>>>
>>>
>>>      I'm unsure about why you need it and if this is the right way to do
>>>      that.
>>>
>>>      What are you trying to achieve?
>>>
>>>      In all case I think I would prefer a clearer way to do this.
>>>      Something like a 'pushopsetup' function or something.

A simple pushopsetup() function that returns a pushop would work for 
this and be pretty clear, I think. Thoughts on this approach, Sean?

>>>
>>>
>>> I've also wanted this before. Use case is extensions may want to change
>>> attributes of pushoperation or pulloperation when they are constructed.
>>> It is sometimes difficult to influence these attributes from just the
>>> function arguments to push() and pull().
> It is exactly as Greg said here.
>
>>> I would still prefer we stop refactoring functions to facilitate
>>> monkeypatching and would institute something more formal, like
>>> https://selenic.com/pipermail/mercurial-devel/2014-September/062046.html. There
>>> was lukewarm reception to that idea. It was not accepted because an
>>> in-tree consumer (probably an extension) was needed. Perhaps we can find
>>> something for 3.7...
> Cool, but we want something in before the freeze.
>
>> pull have a oparg argument that is passed to the pull operation. We
>> should probably go that route if we want to extensibility in push operation.
>>
>> (still wondering smf usecase)
> For remote bookmarks, we want to endow the push operation with some new
> flags. Since push creates the pushop, we want to skip that and create it
> ourselves so we can pass around the new attributes.

Go go into even more detail, we want to be able to set a new attribute 
like 'newbranch' on the pushop. Right now, we're using global state 
inside of remotenames which is ugly and error-prone.

>
> The 'opargs' idea for pullop doesn't go far enough. Currently, there is
> no way to actually use opargs to set new attributes. If you want, we can
> do something like this for pushop / pullop:
>
> def __init__(..., opargs):
>    ...
>    for key, val in opargs.iteritems():
>      settattr(self, key, val)

This sounds more complicated, I like the pushopsetup() idea proposed 
above, but whatever lets us move away from globals would be good.

>
> Thoughts?
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel