← Back to team overview

yellow team mailing list archive

Re: [Merge] lp:~frankban/charms/oneiric/buildbot-slave/use-lpsetup into lp:~yellow/charms/oneiric/buildbot-slave/trunk

 

Review: Approve

Hi Francesco.  Cool!

We'll want to have this merged into the official charm as well (https://code.launchpad.net/~charmers/charms/precise/buildbot-slave/trunk and https://code.launchpad.net/~charmers/charms/oneiric/buildbot-slave/trunk), so some of my comments will come from that perspective.

I think we should keep url for backwards compatibility.  Say that url is deprecated, and it is an error to provide both url and source.  I know it makes the code messy, but I think it is the right thing to do.  I'm happy to have the associated variables named "source" in the code, once the backwards compatibility hacks have happened.  Also, all the examples should use the "source" variants.

In lpbuildbot.yaml, isn't --use-urandom (or similar) an option in lpsetup also?  Or do we always make the urandom->random hack with lpsetup?  If we always do it, we should probably change that, IMO: people will not want (or likely need?) that for their dev machines, for instance.

That's it.  Thanks!
-- 
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-slave/use-lpsetup/+merge/105086
Your team Launchpad Yellow Squad is subscribed to branch lp:~yellow/charms/oneiric/buildbot-slave/trunk.


References