launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Review: Approve

Thanks Ian, love the cleanup and tests. I also didn't know about HTML_PARSER so awesome.

#282/4
I'd make the module name just lp.bugs.duplicate(s). Think of it more like a python module. You have a class in there MarkBugDuplicate, and this module might contain other code later so no sense pegging it tight.

#299
Can we move all the rendering logic over to a render() method? The initializer is really for setting things up, and if it calls this.render() that's cool, but it seems like a lot of work for the initializer to do.

Probably something like:
https://pastebin.canonical.com/70793/

#354
Can we prefix methods that aren't meant as part of the public api with _ as we would in Python code? 

#390
Could perhaps then be setup to use the _bind_events methods from above?

#415
indent the code in the method

#477
Instead of finding the first node to insert before just use prepend on the container
http://yuilibrary.com/yui/docs/api/classes/Node.html#method_prepend

Finally, some of the code has comments with yui doc notes. Can you update the others with at least he basics for method and @private. I'm hoping at some point to start to generate a docs build so that we can see what code we have with basic api to the modules.

http://yui.github.com/yuidoc/

A good example would be the indicator module in app/js/indicator.js

-- 
https://code.launchpad.net/~wallyworld/launchpad/private-dupe-bug-warning-943497/+merge/116248
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.