On Thu, Nov 10, 2016 at 10:55:17AM -0500, Augie Fackler wrote: > On Wed, Nov 09, 2016 at 03:48:39PM -0800, Gregory Szorc wrote: > > On Wed, Nov 9, 2016 at 9:35 AM, Augie Fackler <r...@durin42.com> wrote: > > > > > On Mon, Nov 07, 2016 at 07:13:58PM -0800, Gregory Szorc wrote: > > > > # HG changeset patch > > > > # User Gregory Szorc <gregory.sz...@gmail.com> > > > > # Date 1478573874 28800 > > > > # Mon Nov 07 18:57:54 2016 -0800 > > > > # Node ID 6755052bb245a2fbf30a22c8b4cf5599441fabfb > > > > # Parent 8e2ca8bfa2d0f7c02b6f58b36341b4c4cf7ec0f9 > > > > util: remove compressorobj API from compression engines > > > > > > I like it. Let's chat some about the interface defined in patch 1 > > > (class scope invites questions, and none of those instances actually > > > use the self attribute so I'd like to try and get things demoted to > > > module level unless there's a compelling reason), and then get this > > > landed. Thanks! > > > > > > > It was originally less heavyweight and Pierre-Yves's review feedback > > steered me towards the current implementation... > > > > I agree the classes are essentially singletons and don't provide much > > value. But no matter how you slice it, you need a way to hold references to > > a collection of functions for a particular engine. That's, uh, what a class > > is for. I suppose we could pass function "pointers" to a class's __init__ > > and have it populate attributes. But the base class with stub methods does > > provide some safety and an obvious place to document the API. And the final > > result would probably have the same external API, just N instances of a 1 > > class instead of 1 instances of N classes. > > > > FWIW, I anticipate the zstd engine will need "self" storage. This is to > > facilitate C vs CFFI and lazy module importing and possibly caching of > > certain objects to facilitate faster revlog operations. > > If the zstd thing will need self-storage in an instance, then I'm okay > to take this as-is and clean it up to be less classy if we end up not > needing it.
(Which is to say, queued) _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel