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

Assigned To

Authored By

	Jdlrobson
	Jun 9 2015, 12:20 AM

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

	Subject	Repo	Branch	Lines +/-
	Don't pull in initialisation code.	mediawiki/extensions/MobileFrontend	master	+437 -264
	Hygiene: Modules should be standalone and not pull initialisation code	mediawiki/extensions/MobileFrontend	master	+79 -59

Customize query in gerrit

Related Objects

Mentioned In: T112309: Remove all the cache related FIXMEs
rEMFR541b3eec976f: Don't pull in initialisation code.
rMEXT944895510bf4: Updated mediawiki/extensions Project: mediawiki/extensions/MobileFrontend…
T103027: QUnit should bypass target mode
Mentioned Here: T103991: Nearby pulls in `nav` styling

Event Timeline

Jdlrobson created this task.Jun 9 2015, 12:20 AM

Jdlrobson raised the priority of this task from to Needs Triage.

Jdlrobson updated the task description. (Show Details)

Jdlrobson added a project: Web-Team-Backlog.

Jdlrobson moved this task to Triaged but Future on the Web-Team-Backlog board.

Jdlrobson subscribed.

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

Jdlrobson moved this task from Triaged but Future to 2017-18 Q3 on the Web-Team-Backlog board.Jun 9 2015, 7:04 PM

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

Can't edit patch on mobile but it is https://gerrit.wikimedia.org/r/#/c/212036/

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

gerritbot added a project: Patch-For-Review.Jun 18 2015, 4:33 PM

Jdlrobson mentioned this in T103027: QUnit should bypass target mode .Jun 18 2015, 10:22 PM

Jdlrobson added a parent task: T103027: QUnit should bypass target mode .Jun 25 2015, 9:54 PM

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?

Jdlrobson removed a parent task: T103027: QUnit should bypass target mode .Jun 30 2015, 6:13 PM

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

Jdlrobson moved this task from 2017-18 Q3 to Triaged but Future on the Web-Team-Backlog board.Jul 1 2015, 12:25 AM

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.

Jdlrobson merged a task: T103991: Nearby pulls in `nav` styling.

Jdlrobson moved this task from Triaged but Future to 2017-18 Q3 on the Web-Team-Backlog board.Aug 12 2015, 6:13 PM

• Jhernandez triaged this task as Medium priority.Aug 17 2015, 12:38 PM

• Jhernandez edited projects, added MobileFrontend; removed Web-Team-Backlog.

• Jhernandez moved this task from Backlog to team:other on the MobileFrontend board.

Jdlrobson added a project: Reading-Web-Sprint-55-π.Sep 4 2015, 12:15 AM

Jdlrobson moved this task from Needs Analysis to Code Review on the Reading-Web-Sprint-55-π 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

@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.

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

229449
231069
231630

phuedx moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-55-π board.Sep 9 2015, 10:07 AM

@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 -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-55-π board.Sep 9 2015, 4:45 PM

Jdlrobson moved this task from Code Review to Doing on the Reading-Web-Sprint-55-π board.Sep 9 2015, 8:59 PM

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

Both 231069 and 231630 need attention.

phuedx moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-55-π board.Sep 10 2015, 1:05 PM

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

Jdlrobson moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-55-π board.Sep 10 2015, 4:37 PM

• bmansurov moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-55-π board.Sep 11 2015, 4:21 PM

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 unsubscribed.Sep 11 2015, 7:52 PM

• bmansurov moved this task from Code Review to Ready for Signoff on the Reading-Web-Sprint-55-π board.Sep 14 2015, 12:09 AM

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

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

• Konfused-Kitten mentioned this in rMEXT944895510bf4: Updated mediawiki/extensions Project: mediawiki/extensions/MobileFrontend….Sep 14 2015, 12:24 AM

• bmansurov mentioned this in rEMFR541b3eec976f: Don't pull in initialisation code..Sep 14 2015, 12:24 AM

ReleaseTaggerBot added a project: WMF-deploy-2015-09-15_(1.26wmf23).Sep 14 2015, 1:00 AM

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.

Jdlrobson mentioned this in T112309: Remove all the cache related FIXMEs.Sep 17 2015, 9:24 PM