These should not exist on mw.mobileFrontend
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Define router and overlayManager outside M | mediawiki/extensions/MobileFrontend | master | +64 -47 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Jdlrobson | T86245 mw.mobileFrontend should have a minimal amount of methods | |||
| Resolved | • bmansurov | T86246 Move overlayManager and router as defined modules using M.define |
Event Timeline
Comment Actions
Change 185922 had a related patch set uploaded (by Bmansurov):
Define router and overlayManager outside M
Comment Actions
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!
Comment Actions
@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.
Comment Actions
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.
Comment Actions
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.
Comment Actions
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 :)
Comment Actions
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?
Comment Actions
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?
Comment Actions
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).