Code review comment for lp:~jelmer/bzr/lp-qa-staging

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

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

On 1/17/2011 12:45 PM, Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~jelmer/bzr/lp-qa-staging into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> For more details, see:
> https://code.launchpad.net/~jelmer/bzr/lp-qa-staging/+merge/46515
>
> This adds support for lp://qastaging

     from launchpadlib.uris import LPNET_SERVICE_ROOT
 except ImportError:
     LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/'
+try:
+ from launchpadlib.uris import QASTAGING_SERVICE_ROOT
+except ImportError:
+ QASTAGING_SERVICE_ROOT = 'https://api.qastaging.launchpad.net/'

^- These sure look like things that should be done with getattr() rather
than import hacks. Can we change these to:

from launchpadlib import uris

QASTAGING_SERVICE_ROOT = getattr(uris, 'QASTAGING_SERVICE_ROOT',
     'https://api.qastaging.launchpad.net')

etc.

launchpadlib may be doing import time hacks, but I doubt it.

The rest looks good to me. I would make sure you manually tested it, but
otherwise I'm happy.

 merge: approve

John
=:->

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

iEYEARECAAYFAk011pAACgkQJdeBCYSNAANGrACfc/oUxjzdgBvnVnKj8Cs8VnIf
ZuoAoIIxa+r1h36raJcEdpaKaxUIjz87
=thDm
-----END PGP SIGNATURE-----

review: Approve

« Back to merge proposal