Code review comment for lp:~ian-clatworthy/bzr/cmds-in-userref-585667

Revision history for this message
Martin Pool (mbp) wrote :

On 26 May 2010 17:20, Ian Clatworthy <email address hidden> wrote:
>> Ian, can I ask what prompted this? It was quite deliberately changed
>> to not have any commands unless you come in through the command
>> handler logic.
>
> Bug 585667. Whomever deliberately changed the semantics of this API didn't grep through the codebase to see what would break. :-( Nor did they document it in NEWS (as far as I can tell) so numerous plugins might be broken by this as well.

Hi, that was probably me, sorry. Thanks for catching it. I did grep
for callers but apparently didn't interpret the results well enough.

The point of the change was to let us load less code up front:
generally speaking not to load command classes until they're really
needed, either to run them or to get their help.

We could clean up a bit more the distinction between just getting the
list of names and loading them. I think this change is not
necessarily wrong but perhaps a better one is possible. If
test_import_tariffs passes I don't object to it.

I'll try to look more at this in a bit.
--
Martin <http://launchpad.net/~mbp/>

« Back to merge proposal