← Back to team overview

yellow team mailing list archive

Re: [Merge] lp:~gmb/charms/oneiric/buildbot-slave/charm-tests into lp:~yellow/charms/oneiric/buildbot-slave/trunk

 

Review: Approve code

This branch looks good.  Here are the suggestions I had:

Instead of creating a command to just call it once (starting on
line 15 of the diff):

    -     command('chmod', '+x', script)
    +     chmod = command('chmod')
    +     chmod('+x', script)

I suggest using "run":

    run('chmod', '+x', script)

Instead of doing your own wait-for-the-unit-to-come-up loop in
buildbot-slave.test you can use helpers.wait_for_unit().

We could use a HACKING.txt file like the master.

Regarding (do_not_)test_script: If I'm understanding correctly the
function of the script configuration options, we could test them without
going as far as the openport.py script goes.  For example, we could have
a well-known file (say /tmp/the-slave-setup-script-works) that we verify
does not exist, run a deploy who's configuration specifies a test script
to create the file, and then verify that the file exists.

-- 
https://code.launchpad.net/~gmb/charms/oneiric/buildbot-slave/charm-tests/+merge/92098
Your team Launchpad Yellow Squad is subscribed to branch lp:~yellow/charms/oneiric/buildbot-slave/trunk.


References