[PATCH] mq: defer command wrapping to extsetup
Pierre-Yves David
pierre-yves.david at logilab.fr
Mon Jun 11 04:09:06 CDT 2012
On Sat, Jun 09, 2012 at 11:39:32AM -0400, Matt Harbison wrote:
> Pierre-Yves David wrote:
> >On 9 juin 2012, at 06:33, Matt Harbison wrote:
> >This seems a "big" changes. Can you elaborate in you changeset description what kind of operation are moved from uisetup to extsetup?
> >
>
> Hi Pierre-Yves,
>
> How about:
>
> mq: defer command wrapping to extsetup
>
> mq wraps all commands that are not in commands.norepo, which is now
> performed in this second phase of the extensions setup process.
> This goes against the current best practices on the wiki [1] as far
> as where command wrapping is performed, but follows it regarding
> where global options are injected.
>
> mq needs to be the first layer called when command dispatching in
> order to consistently retarget to the patch repo, regardless of the
> load order of the extensions. This means being the last to wrap the
> command table. Previously, 'hg <extdiff> --mq' would diff the main
> repo unless mq was enabled after extdiff.
>
> [1] http://mercurial.selenic.com/wiki/WritingExtensions
Looks good to me. You should add a (API) flags in the first line.
A small in code comment that explain why wrapping are done in extsetup is
probably a good idea too.
> >It seems to be command wrapping only (which belong to "uisetup"). But this is command wrapping to add a "global options" which belong to "extsetup"?
> >
>
> Yes, and I wasn't sure how to reconcile the two. I had an email
> composed to ask, but the patch was shorter :-) But, does that mean
> import, summary and init should go back to uisetup? I still think
> mq should be the last to wrap those too for consistency, though I
> can't imagine there will be a lot of issues there either way.
I've checked the patch more closely and I did not saw any former content of
uisetup that does not deserve to move to extsetup. I think the patch is fine as
it is.
> >The patch looks good otherwise. It will probably deserve a warning in the next changelog given the amount of extensions that may interact with mq.
> >
>
> Is there something I need to do for the changelog, or is this a note
> to Matt?
The wiki says:
> use '(BC)' to flag backward compatibility changes, use '(API)' to flag major internal API changes.
So just stick an (API) in your first line.
--
Pierre-Yves David
http://www.logilab.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120611/7d5d7c72/attachment.pgp>
More information about the Mercurial-devel
mailing list