[PATCH 2 of 3] upgrade: move descriptions and selection logic in individual classes

Yuya Nishihara yuya at tcha.org
Sun Apr 16 09:18:50 EDT 2017


On Thu, 13 Apr 2017 01:12:15 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1492007645 -7200
> #      Wed Apr 12 16:34:05 2017 +0200
> # Node ID 346b07413137d7f56196941212958df03e55b202
> # Parent  b42c1f35aedd153c77741d54dac50aa6d3ea43e2
> # EXP-Topic upgraderepo
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 346b07413137
> upgrade: move descriptions and selection logic in individual classes
> 
> Our goal here is to get top level definition for all the format variants. Having
> them defined outside of the function enabled other users of that logic.
> 
> They are two keys components of a format variant:
> 
> 1) the name and various descriptions of its effect,
> 
> 2) the code that checks if the repo is using this variant and if the config
>    enables it.
> 
> That second items make us pick a class-based approach, since different variants
> requires different code (even if in practice, many can reuse the same logic).
> Each variants define its own class that is then used like a singleton.  The
> class-based approach also clarify the definitions part a bit since each are
> simple assignment in an indented block.

Since I'm not a fan of using class as a syntactic sugar, I prefer a plain
function that creates an improvement object. But I have no strong option
about that.

> -class formatvariant(improvement):
> -    """an improvement subclass dedicated to repository format
> +class formatvariant(object):
> +    """an improvement subclass dedicated to repository format"""

Nit: it's no longer be a subclass of improvement.

> +    type = deficiency
> +    ### The following attributes should be defined for each class:
> +
> +    # machine-readable string uniquely identifying this improvement. it will be
> +    # mapped to an action later in the upgrade process.
> +    name = None

Confirmed that the formatvariant class doesn't need __eq__ because it won't be
instantiated.

> +class fncache(requirementformatvariant):
> +    name = 'fncache'
> +
> +    _requirement = 'fncache'
> +
> +    default = True
> +
> +    description = _('long and reserved filenames may not work correctly; '
> +                    'repository performance is sub-optimal')
> +
> +    upgrademessage=_('repository will be more resilient to storing '
> +                     'certain paths and performance of certain '
> +                     'operations should be improved')

Missing spaces around '='. The same formatting issues are seen in several
places.


More information about the Mercurial-devel mailing list