Code review comment for lp:~timrchavez/offspring/offspring-sync-milestones-gracefully-handle-buggy-launchpad-api

#1

+ def test_sync_project_milestones(self):
+ """
+ Test milestone syncing does not abort with bad milestone objects.
+ """
+
+ def test_sync_project_milestones_handles_bad_milestone(self):
+ """
+ Test milestone syncing does not abort with bad milestone objects.
+ """

Something's not quite right, they both have the same docstring ;-)

#2

I appreciate that this is a hangover from the previous implementation...but...

+ try:
+ credentials.load(open(config.get("master", "launchpad_oauth")))
+ launchpad = Launchpad(credentials, LPNET_SERVICE_ROOT)
+ except:
+ launchpad = Launchpad.get_token_and_login(
+ 'Offspring Image Build System', LPNET_SERVICE_ROOT, cachedir)
+ launchpad.credentials.save(
+ file(config.get("master", "launchpad_oauth"), "w"))

the bare except should be fixed with something a bit more appropriate...

#3

I'd like to see a test with a successful run of the script...that doesn't have to hit Launchpad, but there should be a test with sync_milestones returning something other than None