yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

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.