| Submitter | Pierre-Yves David |
|---|---|
| Date | April 12, 2017, 11:12 p.m. |
| Message ID | <b42c1f35aedd153c7774.1492038734@nodosa.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/20143/ |
| State | Superseded |
| Headers | show |
Comments
On Thu, 13 Apr 2017 01:12:14 +0200, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org> > # Date 1491860083 -7200 > # Mon Apr 10 23:34:43 2017 +0200 > # Node ID b42c1f35aedd153c77741d54dac50aa6d3ea43e2 > # Parent 3c77f03f16b386940c60af36d6a3ad83bee37ad4 > # 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 b42c1f35aedd > upgrade: introduce a 'formatvariant' class The code looks generally good, but I'll wait for some comments from Greg. A couple of nits inline. > --- a/mercurial/upgrade.py > +++ b/mercurial/upgrade.py > @@ -120,6 +120,23 @@ class improvement(object): > upgrademessage > Message intended for humans explaining what an upgrade addressing this > issue will do. Should be worded in the future tense. > + """ > + def __init__(self, name, type, description, upgrademessage): > + self.name = name > + self.type = type > + self.description = description > + self.upgrademessage = upgrademessage > + > + def __eq__(self, other): > + if not isinstance(other, improvement): > + # This is what python tell use to do > + return NotImplemented > + return self.name == other.name This isn't a bug introduced by this patch, but __ne__ and __hash__ should also be defined. > + def __init__(self, name, description, upgrademessage, fromdefault, > + fromconfig): > + super(formatvariant, self).__init__(name, 'deficiency', description, > + upgrademessage) Better to use the "deficiency" constant, but I don't care much since __init__ will be removed by the next patch.
On 04/16/2017 03:06 PM, Yuya Nishihara wrote: > On Thu, 13 Apr 2017 01:12:14 +0200, Pierre-Yves David wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org> >> # Date 1491860083 -7200 >> # Mon Apr 10 23:34:43 2017 +0200 >> # Node ID b42c1f35aedd153c77741d54dac50aa6d3ea43e2 >> # Parent 3c77f03f16b386940c60af36d6a3ad83bee37ad4 >> # 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 b42c1f35aedd >> upgrade: introduce a 'formatvariant' class > > The code looks generally good, but I'll wait for some comments from Greg. > A couple of nits inline. > >> --- a/mercurial/upgrade.py >> +++ b/mercurial/upgrade.py >> @@ -120,6 +120,23 @@ class improvement(object): >> upgrademessage >> Message intended for humans explaining what an upgrade addressing this >> issue will do. Should be worded in the future tense. >> + """ >> + def __init__(self, name, type, description, upgrademessage): >> + self.name = name >> + self.type = type >> + self.description = description >> + self.upgrademessage = upgrademessage >> + >> + def __eq__(self, other): >> + if not isinstance(other, improvement): >> + # This is what python tell use to do >> + return NotImplemented >> + return self.name == other.name > > This isn't a bug introduced by this patch, but __ne__ and __hash__ should > also be defined. Ah, good catch, do you want a V2 or a follow up ? > >> + def __init__(self, name, description, upgrademessage, fromdefault, >> + fromconfig): >> + super(formatvariant, self).__init__(name, 'deficiency', description, >> + upgrademessage) > > Better to use the "deficiency" constant, but I don't care much since __init__ > will be removed by the next patch. Good catch. (will be in V2/followup)
Patch
diff --git a/mercurial/upgrade.py b/mercurial/upgrade.py --- a/mercurial/upgrade.py +++ b/mercurial/upgrade.py @@ -120,6 +120,23 @@ class improvement(object): upgrademessage Message intended for humans explaining what an upgrade addressing this issue will do. Should be worded in the future tense. + """ + def __init__(self, name, type, description, upgrademessage): + self.name = name + self.type = type + self.description = description + self.upgrademessage = upgrademessage + + def __eq__(self, other): + if not isinstance(other, improvement): + # This is what python tell use to do + return NotImplemented + return self.name == other.name + +class formatvariant(improvement): + """an improvement subclass dedicated to repository format + + extra attributes: fromdefault (``deficiency`` types only) Boolean indicating whether the current (deficient) state deviates @@ -129,20 +146,13 @@ class improvement(object): Boolean indicating whether the current (deficient) state deviates from the current Mercurial configuration. """ - def __init__(self, name, type, description, upgrademessage, **kwargs): - self.name = name - self.type = type - self.description = description - self.upgrademessage = upgrademessage - for k, v in kwargs.items(): - setattr(self, k, v) - - def __eq__(self, other): - if not isinstance(other, improvement): - # This is what python tell use to do - return NotImplemented - return self.name == other.name + def __init__(self, name, description, upgrademessage, fromdefault, + fromconfig): + super(formatvariant, self).__init__(name, 'deficiency', description, + upgrademessage) + self.fromdefault = fromdefault + self.fromconfig = fromconfig def finddeficiencies(repo): """returns a list of deficiencies that the repo suffer from""" @@ -155,9 +165,8 @@ def finddeficiencies(repo): # requirements, so let's not bother. if 'fncache' not in repo.requirements: - deficiencies.append(improvement( + deficiencies.append(formatvariant( name='fncache', - type=deficiency, description=_('long and reserved filenames may not work correctly; ' 'repository performance is sub-optimal'), upgrademessage=_('repository will be more resilient to storing ' @@ -167,9 +176,8 @@ def finddeficiencies(repo): fromconfig='fncache' in newreporeqs)) if 'dotencode' not in repo.requirements: - deficiencies.append(improvement( + deficiencies.append(formatvariant( name='dotencode', - type=deficiency, description=_('storage of filenames beginning with a period or ' 'space may not work correctly'), upgrademessage=_('repository will be better able to store files ' @@ -178,9 +186,8 @@ def finddeficiencies(repo): fromconfig='dotencode' in newreporeqs)) if 'generaldelta' not in repo.requirements: - deficiencies.append(improvement( + deficiencies.append(formatvariant( name='generaldelta', - type=deficiency, description=_('deltas within internal storage are unable to ' 'choose optimal revisions; repository is larger and ' 'slower than it could be; interaction with other ' @@ -202,9 +209,8 @@ def finddeficiencies(repo): for rev in cl: chainbase = cl.chainbase(rev) if chainbase != rev: - deficiencies.append(improvement( + deficiencies.append(formatvariant( name='removecldeltachain', - type=deficiency, description=_('changelog storage is using deltas instead of ' 'raw entries; changelog reading and any ' 'operation relying on changelog data are slower '