Review: Approve bac Hi Francesco,
Thanks for making these fixes. When I tried to get your branch, including having already gotten the pre-requisite branch, bzr warned of a "criss-cross" merge and the diff I have is 195 lines compared to the 266 in this MP. I wonder if something funny has happened. Please pay attention when merging to ensure all goes well. I wonder why this URL script-url: "http://bazaar.launchpad.net/~gary/launchpad/setuplxc/utilities/setuplxc.py"; is referencing Gary's branch, which just changed from Graham's. Why not use the version in the launchpad trunk? If things are in flux, at a minimum we should put up a version under ~yellow so this URL doesn't continue changing. (Gary explained his version hasn't landed yet.) I find the comment at lines 101-103 odd. If that comment refers to what handle_script does, then it should be moved there. It is a nit, and one that irks some people, but PEP-8 says comments should be properly capitalized and punctuated sentences. So at e.g. 145, 159, and 204 those sentences should be capitalized. --bac -- https://code.launchpad.net/~frankban/charms/oneiric/buildbot-slave/merged-run-as-buildbot/+merge/92543 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
