[PATCH 3 of 4 evolve-ext] evolve: extract cleanup logic in the evolve function

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue May 5 18:51:57 CDT 2015



On 05/05/2015 03:50 PM, Laurent Charignon wrote:
>
>
> On 5/5/15, 2:54 PM, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
> wrote:
>
>>
>>
>> On 05/05/2015 02:29 PM, Laurent Charignon wrote:
>>> # HG changeset patch
>>> # User Laurent Charignon <lcharignon at fb.com>
>>> # Date 1430861135 25200
>>> #      Tue May 05 14:25:35 2015 -0700
>>> # Node ID 0af82f6387fce507cd856aa0866534af0be65c98
>>> # Parent  1a06c8c24ea35697471e2108581a6be71773c8e0
>>> evolve: extract cleanup logic in the evolve function
>>>
>>> We are going to need to reuse the cleanup logic when introducing --rev,
>>> so
>>> we extract it in a method to avoid code duplication.
>>
>> Why are your extracting this code into a closure instead of an actual
>> function? Could we have a function instead?
>
> Contrary to _solveone and _handlenotrouble we probably won't need to reuse
> this logic anywhere else, it makes sense only in the context of the evolve
> function. Isn't it?

Yeah but unnecessary closure makes it hard to wrap stuff and read code. 
you better makes it a top level function. We should use closure when we 
actually -need- to capture some context. Not for name-spacing or 
lazyness purpose.

You can see similar effort in Mercurial core `hg log --rev 'desc(closure)'`


-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list