Page MenuHomePhabricator

Deprecate non-injected accesss to ResourceLoaderModule::getConfig
Open, LowPublic

Description

The ResourceLoaderModule::getConfig method falls back to the global service locator. This code was unreachable until recently when two separate issues were allowed to be merge because of this fallback preventing it from failing.

  1. getConfig() is called in the SkinModule constructor for reading wgUseNewMediaStructure. It does so before the Config object is set by the ResourceLoader service. This issue was observed during development, but addressed by working around it in the unit tests.
  2. Also in SkinModule, the value for wgResourceBasePath is read from global state instead of from Config. This has also made the test significantly more complicated than one would like for a test, to the point that the test is as complex if not more so than the code it is testing.

Event Timeline

Change 673309 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Simplify 'features' processing in SkinModule constructor

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

Change 673310 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Fix reliance on global state in SkinModule

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

Change 673311 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove global service fallback in Module::getConfig()

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

Krinkle moved this task from Inbox to Confirmed Problem on the MediaWiki-ResourceLoader board.
Krinkle moved this task from Inbox to Backlog: Maintenance on the Performance-Team board.

Change 673309 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Simplify 'features' processing in SkinModule constructor

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

Change 673310 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Fix reliance on global state in SkinModule

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

Jdlrobson added a subscriber: Jdlrobson.

Can this now be resolved?

Can this now be resolved?

No, the deprecation has neither begun nor completed. There is an unmerged commit which would begin the deprecation.

Change 699433 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] resourceloader: Add missing Module->setConfig() calls in tests and installer

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

Change 699433 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Add missing Module->setConfig() calls in tests and installer

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

Change 673311 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Remove global service fallback in Module::getConfig()

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