Review: Approve

Thanks a ton for the cleanup and rename.

A general note, there's no .html files in here so I just want to make sure that 
the tests are all ok. They're including the same .js files I suppose and 
shouldn't have inline JS that needs use() updates. For a sanity check, just 
want to make sure the same number of tests run in develop/trunk as this branch 
to make sure no tests failed to fire for any reason.

#321
The lazr.editor requirement went away. Was it not needed?

#466
I worry that you've created the namespace lp.ui, but you went from Y.lazr to 
Y.lp.ui.overlay as the extend. Is Y.lp.ui.overlay something that exists already 
as a namespace in prev code? I see that FormOverlay is still under just Y.lp.ui 
and not in the additional subspace of .overlay.

[Edit: looks like this was setup per #637 area of the diff. Should the 
FormOverlay also be in this namespace then?]

#807
Since you changed the NAME I think there might be CSS implications with that. I 
just want to make sure we QA those overlays to make sure that there's not any 
CSS in that particular change that will break while the rest of the CSS is 
updated. There tests might pass while the CSS might be borked a bit.

-- 
https://code.launchpad.net/~jcsackett/launchpad/kill-lazr-3/+merge/123648
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

Reply via email to