[PATCH 3 of 5] histedit: break _histedit function into smaller pieces (add _getgoal and _validateargs)

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sat Feb 13 14:44:39 EST 2016



On 02/02/2016 07:28 PM, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1454345734 0
> #      Mon Feb 01 16:55:34 2016 +0000
> # Branch stable
> # Node ID 4118b5fdc4a1fc9acc4a8f6580edb7ea6dd2adcf
> # Parent  2728f0ff3f167be2489fd8857fac536a4d937883
> histedit: break _histedit function into smaller pieces (add _getgoal and _validateargs)

Thank for the slicing of thi gigantic function into multiple smaller 
one. That was long overdue.

However, we are quite strict about keeping large code movement free of 
other change to reduce the chance of hard to track bug introduction and 
to reduce the chance that the code move get rejected because of the changes.

In this patch you are introducing a new GOAL "enum" that should go in a 
later series.

>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -973,6 +973,70 @@
>       finally:
>           release(state.lock, state.wlock)
>
> +class Goal(object):
> +    CONTINUE = 'continue'
> +    ABORT = 'abort'
> +    EDITPLAN = 'edit-plan'
> +    NEW = 'new'

- We don't use class as namespace (Matt think of them a just an anoying 
way to add a layer in the name iirc),

- we don't capitalise class name,

- we don't use FULL CAPS constant,

Please remove this change from this series and submit it as a followup. 
A more standard form would be something like:

goalcontinue = 'continue'
goalabort = 'abort'
goaleditplan = 'edit-plan'
goalnew = 'new'

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list