[MERGE] add a command for managing the Launchpad user ID.
Martin Pool
mbp at canonical.com
Fri Oct 12 01:35:29 BST 2007
Martin Pool has voted resubmit.
Status is now: Resubmit
Comment:
+
+class UnknownLaunchpadUsername(errors.BzrError):
+ _fmt = "The user name %(user)s is not registered on Launchpad."
+
+
+class NoRegisteredSSHKeys(errors.BzrError):
+ _fmt = "The user %(user)s has not registered any SSH keys with
Launchpad."
+
There are no tests for the formatting of these exceptions.
(You might think that is too trivial to test but it is easy to
get the format string wrong.)
+def get_lp_login(config=None):
+ """Return the user's Launchpad Username"""
+ if config is None:
+ config = GlobalConfig()
+
+ return config.get_user_option('launchpad_username')
Further to Aaron's comment, I think you at least need a docstring
about what should be passed as the config. s/Username/username/
+
+def check_lp_login(username, transport=None):
+ """Check whether the given Launchpad username is okay.
+
+ This will check for both existance and whether the user has
+ uploaded SSH keys.
+ """
+
+ if transport is None:
+ transport = get_transport(LAUNCHPAD_BASE)
+
I think you should explain what transport is meant to be for. If it
is only for testing, it would be more consistent with other tests for
you to hook and then reset the variable -- or at least make call it
_transport.
Also there is already an environment variable(?) saying Launchpad's
address, maybe that should be used here too?
Is there a test/guarantee in Launchpad that ~user/+sshkeys will always
be supported?
Does this give a reasonable error if the server is unreachable?
+ Set the Launchpad ID of the current user::
+
+ bzr launchpad-login $USERNAME
That's not such a good example because it won't work on Windows.
@@ -129,10 +173,11 @@
from unittest import TestSuite, TestLoader
import test_register
import test_lp_indirect
+ import test_account
Since this was written we've decided to expose plugins as e.g.
bzrlib.plugins.launchpad, and not to use relative imports. So can you
change them all to fully-qualified names?
This should be in NEWS.
+ takes_options = [
+ Option('no-check',
+ "Don't check that the user name is valid."),
+ ]
+
+ def run(self, name=None, no_check=None):
+ import account
+ check_account = not no_check
Isn't there an easier way to saay this is a boolean that defaults to on?
+ else:
+ self.outf.write('No Launchpad user ID configured.\n')
+ else:
Maybe it should return 1 for failure?
+ import account
Relative import again.
Aside from that it looks ok.
For details, see:
http://bundlebuggy.aaronbentley.com/request/%3Ca7e835d40710111629n64f361b9w20fe230a0e5a1519%40mail.gmail.com%3E
More information about the bazaar
mailing list