Code review comment for lp:~mbp/bzr/506265-command-deprecation

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/28/2011 03:38 AM, Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/506265-command-deprecation into lp:bzr.
>
> Requested reviews:
> John A Meinel (jameinel)
>
> For more details, see:
> https://code.launchpad.net/~mbp/bzr/506265-command-deprecation/+merge/55030
>
> This doesn't fix bug 506265, but it adds a new Command.invoked_as member, which gets the original name before alias expansion.

I'm a little concerned about the "unable to be suppressed by user alias"
aspect.

I think I really prefer an implementation like:

class cmd_deprecated(Command):

 aliases = ['get', 'clone']

 def run(self, *args, **kwargs):
   if self.invoked_as in ('get', 'clone'):
     official_name = 'branch'
   trace.warning('Command %s deprecated in favor of its official name'
    ' %s' % (self.invoked_as, official_name))
   trace.warning('Please retry with that name')

We might also want to do a trick with self.run_argv_aliases, since we
won't be providing all the flags for every command we are deprecating on
this single command. I don't think it would be very hard to do.

Thoughts?

 review: needsfixing

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2QckEACgkQJdeBCYSNAANa+QCgqG2DGer6DxXVxkU99icEgiuO
5aEAoIoqRLyOlEs2z95NAG206DOL7Z5s
=T4mo
-----END PGP SIGNATURE-----

review: Needs Fixing

« Back to merge proposal