Talk:Requests for comment/Deprecation policy

I would suggest following additions:

• There must be a documented way for extensions to remain compatible with past versions of MediaWiki without using deprecated methods. Usually this is something like if-method-exists-then-use-it-else-use-deprecated-method but can be more complicated.
• When updating bundled or non-bundled extensions, the developer should take the compatibility policy of the extension into account by using the above approach if necessary instead of directly converting to the new way.

Are compatibility policies documented somewhere? (I assume you refer to "extension branch X is compatible with MediaWiki branch X and no guarantees about anything else" vs. "extension master is compatible with all MediaWiki versions going back to the one noted in the infobox".) There does not seem to be any way to find out which extension uses which policy; at a minimum this should be an infobox parameter if we want it to be taken seriously.

Not really, but they should be. But where should these be placed so that it is obvious and easy to check for people? Maybe in the repository itself, as you need to access it to make a patch?

I typically look at the extension's page, e.g. Extension:Flow says "Flow master is only supported with core's master; otherwise, use matching branches". It would probably be nice to make this an infobox parameter to make it easier to specify.

Compatibility with MediaWiki versions can be set using extension.json's "requires" property to indicate the minimum MW version that is still supported.

That still doesn't tell me whether I should leave old code in a BC branch or update the requires property when fixing a deprecation.

Now tracked in the infobox. See https://phabricator.wikimedia.org/T156500#3258508 for details.

I propose to create a summary on the top of the policy to summarize main directives explained in the policy, something like did Legal Department with Privacy policy or Trademark policy. This would emphasize main ideas, and can be a quick-read for casual extensions developers. I quickly re-read the current version of the policy and wrote the following example to give a example of such a summary; obviously it should be more carefully written and phrased, and perhaps written when the final policy is fixed.

« MediaWiki core developers should follow the deprecation process above, upon exceptional circonstances. The main directives are: analyse impacts, document, and advertize. Two deprecation steps are introduced: soft deprecation (add @deprecated annotations) and hard deprecation (emit deprecation notices). Deprecations must be mentioned in the RELEASE-NOTES file and in documentation, and may be advertised elsewhere. Any code in core or extensions reaching deprecated code should be rewritten. Removal of code cannot be done unless it was hard-deprecated since "one" ("two" recommanded) stable versions. »

I added a slightly copy-edited version of your summary to the top, how does it look?

• While GitHub probably provides tools to make searching of code mirrored there easier, I'm wary about mentioning searching GitHub generically even as a "SHOULD".
• Security fixes strike me as a potential exception to this rule. Sometimes something has to be removed without warning if it's discovered to be a security issue.
• Particularly when it comes to class fields, we have many instances where things were previously no-visibility and then someone came through and marked them all public because they wanted to quickly fix "no-visibility" code sniffer warnings. gerrit:234270 cleaned up ParserOptions after one such change, it'd have been annoying to have to wait 2 release versions to deprecate them when only two things in Gerrit were using any of them.
• Other example of removing something without deprecating beforehand:
  • gerrit:236044 effectively removed some methods and fields in User (e.g. User::getPassword() and User::$mPassword). My justification was that the methods weren't even working properly in conjunction with extensions like CentralAuth and the fields should probably never have been public in the first place, and neither had much use in extensions in Gerrit.
  • gerrit:321406 got rid of the public ApiBase::$messageMap. It would have been possible to keep it, but it would have been complicated to be using it as a fallback and even more complicated to make it emit hard deprecation notices. And the public access was only used to hack around a poor interface in some hooks for which an easy alternative was available (but not really documented) since 1.27. The effort just didn't seem worth it.
• A link to a wikipage with the list of extensions and skins bundled with the tarball would be nice. Especially if there are any that aren't used on WMF wikis.

• I assume you're wary because it's not actually open source? Or because it's external? In any case, the inclusion of Github is a "let's bridge the gap and meet extension developers where they are" kind of thing - if there are other sites where there are a lot of MediaWiki extensions being hosted, we should also include it.
• I think security issues would fall under the "Removal without deprecation" section, but it still needs to be justified as the policy states.
• At this point if it's documented as "public" (as opposed to "var"), it's public, and part of the "stable API". Tim brought up something similar during the IRC meeting, so I added "or not reasonable" to the exceptions section. If core developers are fairly certain something is not being used and it's safe to make the breaking change straight away, then OK, do it, but it needs to be documented and announced properly. I think this would apply to the ApiBase::$messageMap example too.
• Will add a link to the list of bundled extensions.

To better highlight that security issues can not follow the removal process, I propose to add a header in the "Removal" section -> see my "pull request" (I wrote it on the page and undid it).

(didn’t see the RfC was in "final call")

I greped git-log and picked up these some examples:

• phab:rMW52167c9: in 1.24 "Remove deprecated Title::userCanRead()" hard-deprecated since 1.19
• phab:rMW39dd076: in 1.24 "Remove Title::escapeFullURL() (deprecated since 1.19)" hard-deprecated since 1.19
• phab:rMWf27203e: in 1.29 "Removed deprecated class RevisiondeleteAction" hard-deprecated since 1.25
• phab:rMWc81540d: in 1.26 "Remove deprecated $wgSpecialPageGroups" hard-deprecated since 1.21 + phab:rMWd06aaa9b20 "Do not allow setting deprecated $wgSpecialPageGroups over extension.json" warning about this config parameter in the tool creating extension.json
• Deprecation of non-RL in Gadgets

In Phabricator, there is the tag "Technical Debt", column "Deprecate / Remove" were examples can be found. I first thought about adding the link on the bottom of the page, but then I saw there is nothing about "centralising" the bugs related to deprecation, so I have been bold in adding it: only recommanding to tag bugs in order to find them more easily, not something like "all deprecation MUST/SHOULD be tracked in the bug tracker", I guess it would be too strict. If I have been too bold, feel free to remove this sentence.

I think that's a good idea, thanks :)

It’s great to encourage the use of @internal tags. Some related suggestions:

• Add a link to either PHPDoc where there is a definition of @internal and/or to the perhaps-future standard PSR-5
• Is @unstable defined somewhere? It’s not in PHPDoc nor PSR-5; if it’s not properly defined, perhaps remove it and only suggest @internal
• On the contrary side, I propose to also encourage the use of @api (PHPDoc, PSR-5). The visibility from PHP (public/protected/private) is not always practical, e.g. public is easier for testing purposes, so explicit documentation could be better than implicit. It would clearly say to extension developers they are encouraged to use some function, and in the longer term it will allow the creation of tools to collect statistics about the number of methods with the various visibilities (and non-documented visibility).

For the last point, an example would be ExtensionRegistry.php :) where, I guess, getQueue() and clearQueue() would be @internal and load() would be @api.

Thanks for this RfC!

The only remark I have is that this does not only affect developers (core and extension) but also sys admins trying to update their MW installation. Updating MW from one version to the next should not result in a blank page just because one of your extensions uses a method that just vanished. It should be possible to first update the wiki and then the extensions without breakage in between.

Thanks for the writeup on this! Seems ripe for discussion. Could you file a Phab task that points to this? I'd do that on your behalf, but I don't think there's any way in Phab to edit the "Authored By" field, and it would be better if the Phab task had your UID rather than mine.

@legoktm, ping

Sorry about the delay, filed as T146965.

Thanks for putting this together, it is much needed!

• WikiApiary.com is a UX nightmare; a more practical list of popular extensions would be nice
• how do developers notify user communities of popular extensions about breaking changes? Phabricator might not be effective; some popular extensions might not have phab tags; some popular extensions are not in gerrit so easy to miss them when reviewing what will break. Should there be a concerted effort to improve that situation? Or some "things you need to do if you want to get deprecation warnings for your extension" guideline?
• MediaWiki core code and functionality MUST NOT trigger hard deprecation warnings... - maybe a better way to put that is MediaWiki core code that triggers hard deprecation warnings must not be reachable from non-deprecated core code using non-deprecated configuration settings. Triggering warnings in B/C APIs for extensions should be fine.
• what counts as PHP API? specifically,
  • do we require depreciation for changes to old "everything is public" classes?
  • do we require depreciation for changes to protected methods? (See T146440 for a recent example of such breakage.) We do not use private visibility that much so that would in effect turn every implementation detail into an API. Can we somehow tell apart classes that you are expected to subclass and ones where you are on your own?

Thanks for the feedback :)

• I added a link to the ExtensionDistributor download stats, which could be used as another data point
• I don't really have a good idea for this. I think for practical reasons it will need to be the latter (things you need to do if you want to get deprecation warnings for your extension" guideline), which is basically 1. Mirror your extension into Gerrit, 2. Have a Phab project where people can report bugs
• Thanks, updated the wording.
• I added a "Scope" section to hopefully clarify that. The only thing I could really think of for subclassing was just making it obvious with a documentation comment, which is more conservative to the core side as effectively everything else is now "final" for the purposes of stability.

See T146440 for a recent example of such breakage - actually not an example of that. But still shows that it is easy to break subclasses with changes that you normally would not consider as changing the API.