Page MenuHomePhabricator

Do not pull initialisation scripts along with JavaScript modules
Closed, ResolvedPublic

Description

All files called init.js make various assumptions about the environment they work in.
This can lead to bugs, for instance Nearby in Vector: T103991

It can also cause tests to fail under certain cases - I haven't dug too deeply into why this issue arises - i suspect maybe there is a module not correctly declaring it's dependencies and some kind of race condition happens but i'd like to stop polluting qunit tests with non-idempotent code.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterDon't pull in initialisation code.
mediawiki/extensions/MobileFrontend : masterHygiene: Modules should be standalone and not pull initialisation code

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 to Triaged but Future on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Jdlrobson.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 9 2015, 12:20 AM

This is blocking @Krinkle patch to enable target agnostic qunit testing

phuedx added a subscriber: phuedx.Jun 18 2015, 3:33 PM

@Jdlrobson: Where's the patch for this? (Since it's in Code Review (non-sprint))

Change 212036 had a related patch set uploaded (by Phuedx):
Hygiene: Modules should be standalone and not pull initialisation code

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

Needs rebasing again... before I do that guys how can I get this patch moved along quicker. What are your concerns about reviewing this? How can I make it easier to review?

Change 212036 abandoned by Jdlrobson:
Hygiene: Modules should be standalone and not pull initialisation code

Reason:
Since no one is helping me move this along I might as well abandon and revisit later.

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

Change 212036 restored by Jdlrobson:
Hygiene: Modules should be standalone and not pull initialisation code

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

Change 212036 abandoned by Jdlrobson:
Hygiene: Modules should be standalone and not pull initialisation code

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

Change 229449 had a related patch set uploaded (by Jdlrobson):
Don't pull in initialisation code.

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

Jdlrobson renamed this task from Do not pull initialisation scripts in the test environment to Do not pull initialisation scripts along with JavaScript modules.Aug 12 2015, 5:25 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson set Security to None.
Jhernandez triaged this task as Normal priority.Aug 17 2015, 12:38 PM
Jhernandez edited projects, added MobileFrontend; removed Readers-Web-Backlog.
Jhernandez moved this task from Backlog to Tech debt on the MobileFrontend board.

This has been dragging for some time now so I want to make sure this patch gets some attention so have pulled it into sprint (low priority compared to everything else).

https://gerrit.wikimedia.org/r/#/c/235776/
https://gerrit.wikimedia.org/r/#/c/229449/

Jdlrobson claimed this task.Sep 8 2015, 3:14 PM
phuedx added a comment.EditedSep 9 2015, 9:17 AM

@Jdlrobson: I'm not sure that pulling something into the sprint without discussing it with the team is the right way to get eyes on it.

Also, what about the other 30 ish patches open against MobileFrontend?

Anyway, 235776 has been merged.

phuedx added a comment.EditedSep 9 2015, 9:25 AM

Here's the list of patches that I think need to be reviewed – they're in the dependency tree:

229449
231069
231630

@phuedx that's fair. I'm concerned with the amount of patches we have against MobileFrontend. This one seemed to be one of the most blocking. How would you recommend handling this better ? Should we have a team Gerrit clean up day?

Jdlrobson moved this task from Doing to Code Review on the Reading-Web-Sprint-55-π board.

Both 231069 and 231630 need attention.

229449 needs attention. The others are minor and depend on it.

I think all the dependencies was confusing what I was trying to do so I made a mega patch (not as scary as it looks once you work out what I'm doing... hopefully)
https://gerrit.wikimedia.org/r/229449

Krinkle removed a subscriber: Krinkle.Sep 11 2015, 7:52 PM

Change 229449 merged by jenkins-bot:
Don't pull in initialisation code.

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

Jdlrobson closed this task as Resolved.Sep 16 2015, 5:03 PM
Jdlrobson moved this task from Ready for Signoff to Done on the Reading-Web-Sprint-55-π board.