Page MenuHomePhabricator

Set position top|bottom for ResourceLoader config to deprecated in extension.schema
Closed, DeclinedPublic

Description

The key "ResourceModules" in extension.schema.v1.json and extension.schema.v2.json should be marked as deprecated since T109837 / I6c21e3e47c23df33a04c42ce94bd4c1964599c7f

Due to use of "additionalProperties": false it must stay for valid extension.json.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 2 2018, 9:32 PM

I have no idea if there is a deprecation way for keys in the extension.schema or if that is needed
When running validateRegistrationFile.php it could shown to the developer that some keys are no longer needed or should replaced. But I have no idea if that is a good way to think about or discussion further. Feel free to hijack the idea.

Legoktm added a subscriber: Legoktm.

I think this would be better if ResourceLoader itself emitted a deprecation warning for these...

Restricted Application added a project: Performance-Team. · View Herald TranscriptMay 24 2019, 11:31 PM
Krinkle added a subscriber: Krinkle.EditedMay 26 2019, 1:14 PM

The position attribute was an optimisation. It did not change functional behaviour. As such, I do not plan to apply any form of deprecation or mandatory migration.

In fact, the feature has been removed already several years ago. Starting deprecation now seems odd, and is imho not a good use of developers time to write, review, release, act on in maintained code, and eventually (when?) to remove the detection again.

If anyone feels particularly strongly about these ignored additional keys, they are free to remove them from code they maintain at their earliest convenience. However, as currently written I'm declining this feature request for ResourceLoader and the code we maintain for it in the ExtensionRegistry.

Krinkle closed this task as Declined.May 28 2019, 8:02 PM

The position attribute was an optimisation. It did not change functional behaviour. As such, I do not plan to apply any form of deprecation or mandatory migration.
In fact, the feature has been removed already several years ago. Starting deprecation now seems odd, and is imho not a good use of developers time to write, review, release, act on in maintained code, and eventually (when?) to remove the detection again.

I suppose it wouldn't be a formal deprecation in the normal sense, the main thing we want to do is emit warnings for when position top/bottom are still configured since they're useless, and at worst confusing if people think they're still being respected. Do you think that adding warnings isn't worth it?

If anyone feels particularly strongly about these ignored additional keys, they are free to remove them from code they maintain at their earliest convenience. However, as currently written I'm declining this feature request for ResourceLoader and the code we maintain for it in the ExtensionRegistry.

Ack. I'll work on that as well then.