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.

-- 
Mailing list: https://launchpad.net/~yellow
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~yellow
More help   : https://help.launchpad.net/ListHelp

Reply via email to