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

Proposed by Francesco Banconi
Status: Merged
Approved by: Gary Poster
Approved revision: 32
Merged at revision: 28
Proposed branch: lp://staging/~frankban/charms/oneiric/buildbot-slave/use-lpsetup
Merge into: lp://staging/~yellow/charms/oneiric/buildbot-slave/trunk
Diff against target: 160 lines (+47/-15)
5 files modified
config.yaml (+9/-2)
examples/lpbuildbot.yaml (+5/-5)
hooks/install (+31/-6)
revision (+1/-1)
tests/buildbot-slave.test (+1/-1)
To merge this branch: bzr merge lp://staging/~frankban/charms/oneiric/buildbot-slave/use-lpsetup
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Review via email: mp+105086@code.staging.launchpad.net

Description of the change

= Summary =

Use lpsetup in place of setuplxc to create the launchpad testing environment
used by buildbot.

== Implementation details ==

Added a new transport, *apt*, that can be used to retrieve and run a script
when the charm install hook is executed.

Renamed the *url* config option to reflect the fact that it can be either
a url or a package name: now it is called, more generically, *source*.

Updated lpbuildbot yaml file to install and run lpsetup.

Fixed a bug preventing extra packages (as listed in yaml config)
to be installed in the charm instance.

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

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!

review: Approve
33. By Francesco Banconi

Backward compatible options.

Revision history for this message
Gary Poster (gary) wrote :

Nice change, Francesco.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches