Thanks for the great review. > > #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. >
Agreed. > #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/ > I've changed to use the standard renderUI and bindUI methods. Of course, when the widget is instantiated, the caller now needs to invoke widget.render() which I've done in bugtask_index. My reason for not using this approach originally is that the widget relied on pre-existing markup on the page and I didn't see the loading of the form as a render operation. > #354 > Can we prefix methods that aren't meant as part of the public api with _ as > we would in Python code? > Yes. > #390 > Could perhaps then be setup to use the _bind_events methods from above? > Since I used bindUI, there's no need to call this explicitly. > #415 > indent the code in the method > Doh. > #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. > Done. -- https://code.launchpad.net/~wallyworld/launchpad/private-dupe-bug-warning-943497/+merge/116248 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
