launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Review: Approve

#30 
Are there better names we can use than key, key_name? I'm not sure how they're supposed to be different. I see later that you're giving it a key in the cache to find the data. How about cache_key and then type_name, type_value?

#307
You don't need to call the superclass for renderUI. It's meant to be implemented by your class.

http://yuilibrary.com/yui/docs/api/files/widget_js_Widget.js.html#l623

#333
Same for bindUI

http://yuilibrary.com/yui/docs/api/files/widget_js_Widget.js.html#l612


#335
Can you just move the check for the privacy_link node into the initializer() of the class and set an attribute that's a kill switch. Then all the code should just check its own ATTR

if this.get('enabled')

or something. One method is checking for a node to determine if it should run, the next method then starts checking things the previous one creates and it's a bit of a stack of cards.




-- 
https://code.launchpad.net/~wallyworld/launchpad/branch-infotype-portlet2-1040999/+merge/121527
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.