[PATCH 2 of 3] upgrade: move descriptions and selection logic in individual classes
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Sun Apr 16 19:27:59 EDT 2017
On 04/16/2017 03:18 PM, Yuya Nishihara wrote:
> 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.
The main motivation to use class is the distinct code (and sometimes
attribute) for each variants, having a "builder" function would make the
code handling of the code much more hacky.
I am not fully happy about the current state, but I do not see a clean
alternative yet. Either I'm missing something in your suggestion or I
find it hackier.
>> -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.
Gah, bad merge resolution.
I can send a V2 or a follow up as you prefer. (given how near the freeze
is, a follow up would seems better)
>
>> + 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.
Yep. It does not needs one.
>> +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.
Good cache, I can send a V2 or a follow up as you prefer.
Cheers,
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list