launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

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.