Page MenuHomePhabricator

MobileFrontend should use upstreamed mediawiki.router
Closed, ResolvedPublic3 Story Points

Description

The MobileFrontend router has been upstreamed. We should now use it as its identical to ours and it's being used by discovery so we will benefit from more eyes maintaining it.

Event Timeline

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptMay 10 2016, 5:46 PM

Change 288005 had a related patch set uploaded (by Jdlrobson):
Hygiene: Switch to using mediawiki.router

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

Jdlrobson triaged this task as Normal priority.May 16 2016, 5:22 PM
Jdlrobson added a project: Technical-Debt.
Jdlrobson set the point value for this task to 3.May 31 2016, 4:53 PM
bmansurov added a subscriber: bmansurov.

This one needs more work. I'm happy to continue reviewing the patch if someone else is up for owning the patch.

@MBinder_WMF: How did an unowned change make it into our sprint?

I'll own the change.

phuedx claimed this task.Jun 8 2016, 2:44 PM

@phuedx I believe this was the task that started the sprint in Code Review (I recall that it was discussed during Prioritization, which fits date-wise). The task history suggests that @Jdlrobson did the tagging and moving.

Change 288005 merged by jenkins-bot:
Hygiene: Switch to using mediawiki.router

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

phuedx removed phuedx as the assignee of this task.Jun 13 2016, 2:51 PM

Modules prefixed with skins. are meant to only put things together and boot up the skin. They can lead to side effects when they are pulled into the test environment so they are not pulled in there.

This separation also reminds us to make classes reusable.

The test_OverlayManager.js code as a result does not load (which was why I left it in mobile.startup) so moving to -1.

Note there are also a few issues in the test for instance it tries to initialise an instance of OverlayManager.

Change 294246 had a related patch set uploaded (by Jdlrobson):
Restore OverlayManager tests and move back to mobile.startup library

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

@phuedx I believe this was the task that started the sprint in Code Review (I recall that it was discussed during Prioritization, which fits date-wise). The task history suggests that @Jdlrobson did the tagging and moving.

Correct this was always intended to be in this sprint and we talked about it in prioritisation. I commented on the patchset "(Feel free to take over this patch and any other one of my patches in my absence. Consider this permission to edit.)" :)

Modules prefixed with skins. are meant to only put things together and boot up the skin. They can lead to side effects when they are pulled into the test environment so they are not pulled in there.

We should take the time to document this (or have we already?) I still don't quite understand the naming scheme – e.g. I tend to look for index(.js) or init(.js) files when looking for initialisation scripts – but if the convention is documented, then neato!

The test_OverlayManager.js code as a result does not load (which was why I left it in mobile.startup) so moving to -1.
Note there are also a few issues in the test for instance it tries to initialise an instance of OverlayManager.

Ta.

Change 294246 merged by jenkins-bot:
Restore OverlayManager tests and move back to mobile.startup library

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

bmansurov closed this task as Resolved.Jun 15 2016, 1:53 PM
bmansurov claimed this task.

Closing as resolved as this is tech task and the follow up patch has been merged.

bmansurov removed bmansurov as the assignee of this task.Jun 15 2016, 1:53 PM