Code review comment for lp:~nmb/bzr/switch-uses-directories

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

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

Neil Martinsen-Burrell wrote:
> Neil Martinsen-Burrell has proposed merging lp:~nmb/bzr/switch-uses-directories into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #495263 switch -b doesn't respect directory services
> https://bugs.launchpad.net/bugs/495263
>
>
> This fixes the bug that switch won't use directory services if the name doesn't contain a '/'. There is a new test that fails for the existing bzr.dev and passes with the change. The rest of the switch tests pass with this change as well.
>

As long as the existing functionality is preserved (which seems to be
the case), the change seems fine to me.

However this:
+ directories.register('foo:', FooLookup, 'Create branches named
foo-')

Looks like it leaves a permanent entry in the directories registry. So
I'm thinking you need to self.addCleanup() some function to remove that
value. Something like:

self.addCleanup(directories.remove, 'foo:')

 review: needs_fixing

John
=:->

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

iEYEARECAAYFAksia2MACgkQJdeBCYSNAAO8KgCcCylUrYT+lRDwgQezuwOgLgky
fDgAoIZTZZbrcC+cflBAFBl6w2JnVwhC
=QBAH
-----END PGP SIGNATURE-----

review: Needs Fixing

« Back to merge proposal