> 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.
I'll double check, but a grep for lazr only turned up a reference to a lazr/lazr.js (which was irrelevant, since the new ui/ui.js file was also included). I deleted that line. > #321 > The lazr.editor requirement went away. Was it not needed? Correct; it's not the only place where there are unneeded modules in the required statement, but I decided that I would tackle those another day. > #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. You're right, it should just be lp.ui.PrettyOverlay, not lp.ui.overlay. I've corrected that. > #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. That's a test file; the subclass only exists in the test, which is why I was comfortable changing it. -- 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
