Page MenuHomePhabricator

Move overlayManager and router as defined modules using M.define
Closed, ResolvedPublic

Description

These should not exist on mw.mobileFrontend

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Readers-Web-Backlog.
Jdlrobson moved this task from Incoming to 2015-16 Q4 on the Readers-Web-Backlog board.
Jdlrobson added subscribers: Aklapper, Jdlrobson.
bmansurov set Security to None.
gerritbot added a subscriber: gerritbot.

Change 185922 had a related patch set uploaded (by Bmansurov):
Define router and overlayManager outside M

https://gerrit.wikimedia.org/r/185922

Patch-For-Review

Change 185922 merged by jenkins-bot:
Define router and overlayManager outside M

https://gerrit.wikimedia.org/r/185922

Jdlrobson closed this task as Resolved.Jan 19 2015, 11:11 PM

Hey guys,

Just a quick reminder to make sure you're checking Zero* extensions (or doing some big find/replace, or automated tests, or something) when doing syntax changes like this. A piece of our code that included M.router.route(...) broke on what I believe was this change and we had no idea until today. We still have some additional automated tests to setup on our end too obviously ;)

Just ping me if you ever feel like some syntax change might affect us and I can check it out (I'm also open to other (read: "better") ideas if anyone has them). Thanks!

@jhobs, sorry about it. We are constantly changing the code these days. Please keep an eye on the MobileWeb project here. I'll also try to ping you if something like this changes again.

Yeah, no worries, we're used to this happening. Just throwing out a
friendly reminder when it does :) I try to do my best to keep an eye on the
changes you guys make too.

Zero tests now run in MobileFrontend as part of Jenkins on a +2. If something broke it means your tests in Zero are inadequate/don't exist! :-)

Make sure everything in Zero has some tests to stop this kind of thing happening again.

Oh yeah we have very few tests and definitely need a bunch to avoid this kind of thing. I'm planning to spend a big portion of this quarter writing a lot of these, but I just meant in the meantime if it ever comes to mind that a syntax (or similar) change might affect other extensions, a ping would be much appreciated :)

I'm more than happy to get some tests written with you. Shouldn't take more than 2 hours to do this at most. Want to pair on this?

I'm more than happy to get some tests written with you. Shouldn't take more than 2 hours to do this at most. Want to pair on this?

I think the tests are fairly straightforward, it's more of a matter of finding the time to do them. I hope to get them out of the way later today, but if I don't then let's sync up and pair on this on Friday while I'm still here (or next week, remotely).