[PATCH 1 of 4 V2] flagutil: introduce a flagprocessorsmixin class
yuya at tcha.org
Wed Sep 4 19:43:16 EDT 2019
On Mon, 2 Sep 2019 15:03:00 +0200, Pierre-Yves David wrote:
> On 8/31/19 5:02 AM, Yuya Nishihara wrote:
> > On Sun, 11 Aug 2019 13:10:47 +0200, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david at octobus.net>
> >> # Date 1565219568 -7200
> >> # Thu Aug 08 01:12:48 2019 +0200
> >> # Node ID e6d22ec889461dbe819e0dbf4d2f0167736b2416
> >> # Parent 6d61be152c5515fed315885f6c9cf9defe73de71
> >> # EXP-Topic flag-processors
> >> # Available At https://bitbucket.org/octobus/mercurial-devel/
> >> # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r e6d22ec88946
> >> flagutil: introduce a flagprocessorsmixin class
> > CC +indygreg
> >> diff --git a/mercurial/revlogutils/flagutil.py b/mercurial/revlogutils/flagutil.py
> >> --- a/mercurial/revlogutils/flagutil.py
> >> +++ b/mercurial/revlogutils/flagutil.py
> >> @@ -78,3 +78,74 @@ def insertflagprocessor(flag, processor,
> >> msg = _("cannot register multiple processors on flag '%#x'.") % (flag)
> >> raise error.Abort(msg)
> >> flagprocessors[flag] = processor
> >> +
> >> +class flagprocessorsmixin(object):
> >> + """basic mixin to support revlog flag processing
> >> +
> >> + Make sure the `_flagprocessors` attribute is set at ``__init__`` time.
> > I feel a plain function taking (revlog, flagprocessors, ...) is simpler
> > than the mixin class which implicitly depends on revlog attributes. But
> > maybe I'm biased. I don't like class inheritance in general.
> Are you suggesting (revlog, currently_supported_flags, …)? If so, I am
> afraid this would lead to wrong API usage.
> It is important that the
> revlog bear the supported flags to help enforcing extension/config
Anyway, the user of the mixin would have to set self._flagprocessors
properly. I don't see much difference between mixin + attributes and
passing it as a function argument.
> In all case, can we move forward with this series and adjust in a follow
> up, I have a lot of other changes waiting on this ?
If someone else agree with this change, please go ahead and queue this.
I only have a _minor_ concern, and I don't have expertise around this code.
More information about the Mercurial-devel