Review: Approve

Thank you Gary for this nice branch.


100     +        # This is naive; we can make it more sophisticated (e.g., 
allowing
101     +        # escaping dollar signs) if we discover we need it.  For now,
102     +        # simplicity wins.
103     +        replaced_args = args.replace('$INSTALLDIR', buildbot_dir)

We could also do something like::
    
    formatted_args = args.format(**config)
    
And then, in the config file::

    script-args: "-u buildbot -e [email protected] -f 'Launchpad PQM' 
{installdir}"

It's as naive as your version, but maybe a little more generic?


60      -    command('hg', 'clone', source, target)
61      +    run('hg', 'clone', source, target)

Thank you for fixing all the script retrieval functions.
I have to make a note to myself: command helper returns a function!

Also thanks for your change on how the su context manager retrieves the user 
home directory: 
please see the comment on the master MP for a suggestion on how to fix the 
exceptions problem too.


I've seen now buildslave is created in the install hook (great IMHO), but the 
same code is still present in config-changed: this could generate errors 
(buildslave created twice?), and requires further investigation. 
-- 
https://code.launchpad.net/~gary/charms/oneiric/buildbot-slave/run-as-buildbot/+merge/92400
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