launchpad-reviewers team mailing list archive

[Leonard Richardson <leonard.richardson@xxxxxxxxxxxxx>]

This looks good. Just some nitpicky stuff:

On the web service, getByName is a named operation (or "operation"), not a method. Calling it a method makes it look like a local method call.

'Our "bob" builder was retrieved using the no-priv user, and he does not..' The "he" is ambiguous. s/he/no-priv/.

Do you want to iterate over a *sorted* list of attribute names? bob.lp_attributes is ultimately dependent on the order of attributes in the IBuilder definition. Something to think about.

You might want to distinguish between cprov's launchpad and no-priv's in the variable names. 'launchpad' and 'cprov_launchpad' or 'privileged_launchpad'.

-- 
https://code.launchpad.net/~julian-edwards/launchpad/api-expose-builders/+merge/39379
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/api-expose-builders into lp:launchpad/devel.