[PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

Augie Fackler raf at durin42.com
Tue Mar 21 15:33:27 EDT 2017


On Tue, Mar 21, 2017 at 3:29 PM, Jun Wu <quark at fb.com> wrote:
>> I feel obligated to remind you that we don't offer any stability
>> promises on internals, so if this is a righteous cleanup (using req or
>> similar instead of ui), it's probably worth doing in any case.
>
> I know. In theory we can also drop "extsetup" and only keep "uisetup", or
> drop the support for "extsetup()" without the ui argument. But I guess we
> didn't do that for a reason.

I'm probably going to nibble around dropping support for extsetup
without ui in Python 3. Seems worthwhile as an incremental nudge.

>
>> Note that we're coming up on an interesting moment when we'll know
>> extensions required significant cleanup anyway (python 3), so we might
>> want to evaluate other internal structure cleanups while we're
>> breaking everyone anyway.
>
> Even if we have done replacing "ui" with "req" everywhere, I don't think
> that's better - the request object practically becomes the new "ui" object
> with all kinds of features.
>
> If I guessed correctly, what you want is a new layer inserted, like:
>
>     We have a "root" object. "root" could be either "ui" or "request".
>
>     To access configs, use: root.config.get, root.config.set, and there is
>     no root.setconfig.
>
>     To access I/O related features, use: root.io.fin, root.io.fout, ...,
>     there is no root.fin.
>
>     Just split ui functions to subobjects.
>
> My point is the "root" object does not have to be "request". It can be done
> using the "ui" object incrementally, using a similar approach like what we
> did for "repo.join -> repo.vfs.join". It's more realistic.

Sure, that sounds fine too.

>
> Anyway, the long-term refactoring shouldn't block this simple patch. And I
> think it's reasonable to add "ui.args" for now.

In abstract, this is how we get god objects. I think in this
particular case the progress might be worth the change, but in general
I'm totally willing to block simple patches if they add bad conceptual
layering violations that make future maintenance harder.

(I'll defer to Ryan's part of the discussion for now, and will look at
this series again when there's a resend or some other indication it
needs my attention.)


More information about the Mercurial-devel mailing list