← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~allenap/maas/try-out-saucelabs into lp:maas

 

> In src/maastesting/saucelabs.py, I beseech you to break down the
> methods of SauceConnectFixture further.  I find them hard to read:
> the intent of a given bit of code often becomes clear only after
> going through its details.  Things get much easier if intentions are
> stated up front and the code implemented them, e.g. through
> well-named functions.

I've added some comments, but I don't really see that the functions
are too long. They're named appropriately - start and stop - and
that's what they do. Perhaps they look long because I like to break
lines and indent more than most people?

> 
> In SauceOnDemandFixture.__init__ it's as if you're playing a shell game with
> self.capabilities.  If you're going to have an immutable property in the
> class, and a modified copy in the object, then why not call the class one
> default_capabilities?  It would be easy for someone in a hurry to break this
> in a way that's hard to debug.

Done.

-- 
https://code.launchpad.net/~allenap/maas/try-out-saucelabs/+merge/106217
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/try-out-saucelabs into lp:maas.


References