Code review comment for lp:~sidnei/lazr-js/combo-service

Revision history for this message
Māris Fogels (mars) wrote :

Hi Sidnei,

This looks good. If you have to, I think you could land it as-is, but I do have some comments about the branch:

• Nice formoverlay example in combo.html. It is very straight forward
• Why did some of the lazr module dependencies get updated?
• Do we need to include the css* module dependencies, or can they be inlined on the page? I worry about the two approaches confusing developers.
• test_extract_skinnable() and test_extract_skinnable_with_lazr_conventions() look like copy-n-paste code. Can the common parts be put into a parameterized method?
• Use Y.one() instead of Y.get()

I think we can define a global LAZR() function in lazr-meta to replace YUI(), but that can be saved for another branch.

Maris

review: Approve

« Back to merge proposal