Talk:Stable interface policy/Archive 2

For soft deprecation we say:

"he deprecation MUST be mentioned in the relevant RELEASE-NOTES"

There is no requirement for hard deprecation but @Jdforrester (WMF) pointed out in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1081276 we often update RELEASE NOTES to emit deprecation warnings.

I propose we add the following text to the hard deprecation section:

"When transitioning from a soft to hard deprecation, the RELEASE_NOTES must be updated to reflect the function now emits deprecation warnings."

I'd probably recommend instead:

When upgrading a "soft" deprecation to also start emitting deprecation warnings in a different release to the original deprecation, the RELEASE_NOTES file SHOULD be updated to reflect this.

There are circumstances where it's not strictly needed, and it's only needed if the original deprecation was in a previous release.

While working on a frontend equivalent of this policy, the names "provider" and "consumer" were proposed. In this document we currently have "author" and "user" to mean the same.

These words seem overloaded given an "author" could mean many things - such as editing documentation, editing wiki pages. A "user" could be interpreted as a user of the code.

How about we change all instances of "author" to "provider" and all instances of "user" to "consumer" ?

These words seem overloaded

This document is addressed to a software development context. Do you think there is ambiguity in this context for who the author of a code interface is, or who the user of a code interface is?

In your experience browsing GitHub issue trackers and npm READMEs, do you find that people don't often use and understand terms like "author" and "user", or that these caused confusion? Perhaps an alternate set of terms is more often there that we could borrow from?

If we remove context from the equation, I believe it would be very challenging to find a term that is well known (i.e. not too novel, or unknown to ESLs), easily understood, and ambigious regardless of context.

The word "consumer" seems certainly ambigious. Like "user", a "customer" often refers the end-user (possibly more often?). In my own experience, I find developers of open source software more often refer to themselves as "users" than as "consumers". (Most often would be "developer", but I mean cases where another term is used to disambiguate.) The term "consumer" might be a bit too corporate-y for FLOSS culture? Businesses and commerical enterprises refer to their users as customers.

Companies also tend to think of, and present, themselves as "provider" when they create software or services. This too carries a similar ambiguity. E.g. software consultancies that build WP or MW plugins, WordPress and MediaWiki hosting providers, companies like BlueSpice. I imagine most every developer working as user of MediaWiki core to build an extension, is providing something to someone. Even if merely as hoster of your own wiki with a custom extension, you'd be the site/content provider building on MW software. If I drop "Who is your provider?" in an unspecified technology-adjacent context, I think one tends to think first of web hosting, ISP, and telecom companies; Not easily would I think of the author of one of our npm dependencies, from whom I use the stable interface.

I'm not arguing that "provider" is unclear, but I do think, if we ignore the document context, that it is certainly ambigious. I feel the term "provider" also feels a bit less personal and human than an "author".

Note that we use term author a fair bit in our technical writing when we talk about ourselves as the authors (or "maintainers") of software, including AUTHORS.txt attribution, Git commit authors, author credits from extension.json for Special:Version, and more in search.

Class aliases are basically deprecated names for classes. I think that the deprecation process should apply for them, so it is clear when we can remove them. But there is no good way to emit deprecation warnings...

For things that can't be warned about, we generally do a 'best efforts' approach, looking through codebase through Code Search and determining how likely it is that people will be disrupted (and writing patches to migrate where the codebases are current enough to pass CI). Not sure that this needs formalisation?

Since 1.39, the installer will warn about the use of deprecated settings. But this only works as long as the declaration of the config setting is kept. Since people often upgrade from old versions after many years, it seems useful to keep the declaration around for a long time.

The old setting can however be ignored. The deprecation warning can just say that the setting no longer works. We just need to declaration in place so the installer tells the user.

Maybe instead of using the "deprecated" flag in the schema, we should have an "obsolete" flag for settings that are no longer supported? We could keep them indefinitly.

This is a great idea, though it seems like we could do both? Warn on deprecation and for obsolete settings?

Yes, of course, it should do both! But deprecated settings would still work, while obsolete settings would be defunct. The idea here is to make the distinction explicit, and allow backwards compatibility code for old settings to be removed without breaking old installs without explanation. Being able to declare settings as obsolete will allow us to break old installs WITH explanation ;)

(I just realized I have been editing here with two accounts - full disclosure: User:Duesentrieb is also me, that's my private account)

I made a patch that introduces the an "obsolete" marker into MainConfigSchema, and uses it in the updater and structure tests: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/838769

@DKinzler (WMF), I agree with you on the "obsolete" flag. It will inform 3rd party users to cleanup their LocalSettings.php and remove what is no longer needed for their installs (or update to the new ways of doing things).

Since we can't know what outside user LocalSettings.php looks like, this a good strategy to keep things clean on their own side too, which after sometime, cleaning up these configs from core would be safer.

But the thing is, how long do we want to keep the deprecated configs? 2, 5 or 10 years before completely removing?

With this mechanism in place, we could start to remove backwards compat code for deprecated settings more agressively, after or two releases, like other deprecated code. The declaration of the obsolete setting would need to be kept for as long as we support upgrading from a revision where the setting was not deprecated (but not before two LTS releases from now, because older versions don't emit warnings).

I.e. if in 1.44 we drop support for upgrading from 1.35, we can remove all obsolete config that has been deprecated since at least 1.35. This would be safe since when upgrading to 1.44 from a version > 1.35, you'd see the warnings. And when upgrading from an older version, you'd have to upgrade to a version < 1.44 first, so you'd also see the warnings.

Since we currently guarantee updates across two LTS releases, and we do an LTS release every other year, this would come to four to five years from deprecation to full removal. The b/c code could already be removed after about a year, though, moving the seeting from deprecated to obsolete.

Something that more interactively helps sysadmins when upgrading to know what config they need to update/remove would be really great, rather than hoping that people carefully read the upgrade notes. Great plan.

@Jdforrester (WMF), you captured it very well and I like how you put it. In this case, we're guiding the customer/end-users on how to configure their wikis and what to remove which is no longer needed for/after upgrades.

I think a lot of sysadmins will be happy to see this feature (as here we're putting MW users again on the front line) and it'll make their life a whole lot easier when managing those giant LocalSettings.php files :)

A recent deprecation following this process caused problems in the translatewiki extension (context https://phabricator.wikimedia.org/T310017#8236113)

The guide currently says: > Developers or teams deprecating code SHOULD actively support removal of usages in code in the MediaWiki ecosystem, especially in popular extensions. This includes making maintainers aware of the deprecation as well as creating or reviewing patches removing usages.

"popular extensions" is quite a vague term here, as it's not clear what's popular. Codesearch currently shows results for many extension/skins that are unmaintained or pinned to older MediaWiki versions, so its often hard to identify which skins.

"active" is also vague here. In the case of T310017 work was still ongoing to support removal of usages but a translatewiki deployment happened somewhere in the middle of that process.

Questions:

• Should TranslateWiki installed extensions/skins be treated as "Wikimedia deployed" code and block hard deprecations?
• If not should codesearch provide a tab in addition to "Wikimedia deployed" e.g. "Translatewiki deployed"
• Could we provide more guidance about what we class as popular extensions/skins with some kind of criteria e.g. used on X site, Y+ downloads?

Should TranslateWiki installed extensions/skins be treated as "Wikimedia deployed" code and block hard deprecations?

No, I don't think so. Though we're fans of TWN and fund them, as well as using them, they aren't part of our ecosystem (and shouldn't be forced to follow our processes and all the rest).

If not should codesearch provide a tab in addition to "Wikimedia deployed" e.g. "Translatewiki deployed"

Sure, that's pretty easy to add if that would be useful.

Re: popular extensions, we used to have this dashboard for things in ExtensionDistributor/Gerrit, but it's timing out right now. WikiApiary has been unreliable for years, but that used to be another good source of data.

SMW is a weird spot in which it is an incredibly popular extension, but because development is on GitHub (requires use of proprietary software) and it has no code of conduct (and historically has been toxic/unfriendly) I would not even suggest recommending or asking people to file tickets there. I think that is entirely on the SMW maintainers/community to fix.

Overall I do think TWN is a good bellwether for how people use MediaWiki in the wild, but I don't think it should be codified in policy.

Widening the types of parameters is allowed by the stable interface policy, as it it does not break any callers. Example:

- public function addLink( Title $title, $id = null ) {..}
+ public function addLink( LinkTarget $link, $id = null ) {..}

This can pose an issue for developers of extensions and skins, who want to maintain compatibility for multiple versions of MediaWiki core: when refactoring or writing new code, we may accidentally use LinkTarget (with the above example) even though it will not work with older versions of MediaWiki. Detecting these issues is difficult due to low(er) testing with older versions of MediaWiki compared to the latest version.

For other things, that may be backwards incompatible, we can check the @since tags for the relevant classes/methods/constants/etc. For this kind of compatibility issue there is currently no standard way to check other than reading the source of each supported MediaWiki core version.

I propose that we mention these kind of type changes in the parameter documentation. IDEs like PhpStorm will show this documentation when writing code, which makes it easier to avoid introducing these kind of breakages accidentally.

"Long Term Support" means WMF provides bug fixes and security fixes for two years. It provides no guarantees about backwards compatibility, and it never was intended to do that.

The policy of "removing deprecated code after 1 major release (or 2)" has a really MAJOR problem. It has to do with extensions. With extensions that are not used by MediaWiki (and maybe not used by a lot of people), the extensions may only get updated during LTS revisions. When every single major release that is not a LTS release can have breaking deprecated code, when a programmer comes along and tries to bring old extensions up to date (usable with the new LTS), there is no central "checklist" (or anything) indicating what exactly has changed. Even a list would be helpful. Then at least somebody can create a utility (maintenance file) that can be run against an extension to determine what things need to be updated to be compliant with the new LTS. Specifically, I am talking about third party extensions that run in 1.31, but do not run in 1.36 (for a variety of reasons). Some of which just kind of scrapped by when going from 1.29 (or less) to 1.31.

The whole point of a LTS is so that a company (or third party extension developer) does not need to worry about all of the little changes that happen between LTS version and can just focus on the LTS versions. But it does not seem as though the development team at MediaWiki is paying attention at all to people trying to move from 1.31 LTS to 1.36 LTS.

There needs to be a maintenance function that can at least point out to a third party developer which functions calls (or hooks) are deprecated. Something like "extensionLintChecker.php" and "skinLintChecker.php" for skins. Something that will produce a report saying "File:myextension.php" is using "Deprecated function "nogo()". Replaced by "yesnow1() and againnow()" (or something to point the developer in the right direction to fixing the extension.

It's probably not what you're looking for, but each version's release notes includes a list of deprecated (and removed) code features.

Also most extensions are checked via https://codesearch.wmcloud.org/search/ in general, so this can only rly happen to extensions that have not been added to that. That service is managed https://gerrit.wikimedia.org/g/labs/codesearch and you can also file a phabricator ticket to get extensions added to it.

"LTS" is a commitment about MediaWiki itself, including the skins and extensions shipped in the "tarball". It's not a general commitment for all Wikimedia-authored extensions, and certainly not by the hundreds of independent extension authors.

If you're using an extension that doesn't bother to keep current with MediaWiki except for LTSes, I'd very strongly counsel you to stop using it and find an alternative.

"Long Term Support" means WMF provides bug fixes and security fixes for two years. It provides no guarantees about backwards compatibility, and it never was intended to do that.

Extension developers who choose to only update their extension for LTS releases may see their extension not working with intermediate releases. If that was not the case, we couldn't make any "major" releases between LTS releases. The very definition of a major release is that it breaks backwards compatibility - otherwise, it would be a minor release.

The policy currently states:

• Obsolete behavior MAY be removed right away if it appears to have never been used within the Wikimedia maintained code and the MediaWiki ecosystem (except in the repo that defines it), and seems unlikely to be used elsewhere.

This misses the point that code that has been in use only in the repo it is defined in, and perhaps in extensions maintained by the same entity, should allowed to be removed.

Perhaps this point should be rewritten as follows:

• Obsolete behavior MAY be removed right away if it appears to have never been used within the MediaWiki Ecosystem in a repository other than the one it is defined in. Even then, it MAY be removed right away if it has only been used in repositories maintained by the same entity.

Also, perhaps it would make sense to clarify how this provision interacts with another part of the policy, namely:

• If it is not reasonably possible for the deprecated code to emit deprecation warnings, hard deprecation can be applied by announcing the removal on wikitech-l in a timely manner.

Taken together, this means that certain breaking changes may only require an email to wikitech-l and a brief wait. This may be useful e.g. when it is desirable to change the return type of a method in a way that does not break any known callers: there is no way to deprecate a return type, so if nothing seems to rely on the old type, it's ok to change it after asking about concerns on the mailing list.

Wording clarified by James.

The policy seems to suggest or assume that being @stable and @deprecated are mutually exclusive. Some stability types explicitly state this (such as stable to call and stable to type), in others it seems to be implied (e.g. stable to extend: Only classes that are marked @stable to extend).

To make this clearer, I suggest adding a bullet point similar to the following to the section on soft deprecation:

• Soft deprecated code SHOULD NOT be marked stable. Any @stable annotations should be removed at the same time that a @deprecated annotation is added.

And maybe also this to the section on hard deprecation:

• Hard deprecated code MUST NOT be marked stable.

I agree that they are mutually exclusive and that @stable should be removed upon deprecation. If this gets added to the official policy, it might be worth having a codesniffer warning for both @stable and @deprecated in the same doc block.

I agree that we should remove them on deprecation, but perhaps in our @deprecated tags we should explicitly say that it used to be stable to extend/etc.?

Hm, I just noticed this in Stable interface policy § Add guarantees:

The @stable annotations can be followed by a Deprecated since segment to indicate that a particular use of the class or method is currently deprecated. This can be used to indicate that extensions should no longer subclass, but may still call public methods. This guruantee may then be removed in the next release. Note that there is currently no hard-deprecation for the removal of stability guarantees.

/**
 * @stable to extend Deprecated since 1.35
 */
class Foo {
    /* … */
}

Not sure what that means for this proposal…

Hmm. Perhaps @DKinzler (WMF) can suggest how he saw this working?

I think it should work as follows:

When the intent is to remove, then `@stable` should just be replaced by `@deprecated`. No code should use it.

If the intend is to keep but make internal, `@stable` can have a "deprecated since" qualifier added. This prevents internal use of the affected code from being flagged as deprecated by IDEs and other tooling, while giving extensions authors at least some warning. It's basically "deprecated for public use but ok to use internally".

Thanks, that seems reasonable. I've tweaked the wording; hope this is correct and uncontroversial?

Yes, thank you!

The policy currently states:

• Deprecations and removals SHOULD NOT be performed shortly before a release branch or between release candidates, to give extension authors time to fix any issues that may arise, and avoid broken snapshots of extensions.
• Deprecations and removals MUST first take place on the master branch. Deprecations and removals SHOULD NOT be backported to release branches or release candidates.

It seems like the first bullet point is redundant and misleading: we can still do deprecations on the master branch while release candidates are being prepared. We just shouldn't add deprecations to the branch that is going to be released. Since that is already covered by the wording in the second point, I think the first point should simply be removed.

Agreed on the point about "being prepared", but the main part should stay. It's really unhelpful to make large breaking changes (which deprecations are, in effect) a few days or hours before a branch is cut, and doing so just afterwards can make backports very hard.

Should we be more specific about what "shortly" means? A week should do, right? We'd need some kind of official "pencils down" warning for this to work.

No, not just a week. I'd recommend a month before and after the branch point, generally. This isn't about Wikimedia staff fixing things (where a week's notice is very tight but might be OK), this is about the wider ecosystem, right? People might not even notice that their extension no longer passes CI for a week or two after the deprecation, let alone fix it.

That would mean not making deprecations in two out of six months. That seems like a massive impediment. Can't we just run CI for all of them?

No? Running >>US$1k worth of CI CPU time for every patch to MediaWiki core, and then doing something with that result, would be a massive change to our CI budget and expectations for developers. (If a patch in core causes a failure in an extension, how is that communicated? To whom? How do maintainers find out that they need to do something? How do core developers not end up deluged by false positive "failure" messages from the dozens of extensions that are broken already? Etc.)

Also, this is just documenting current expectations, it's not a change to the current reality, AIUI?

Not doing deprecations for a month before and after a branch cut is definitely not current reality, and never has been as far as I can remember.

My suggestion was to run CI on all of them when doing the rc0 branch, not on every patch. My hope is that this would result in extension maintainers noticing and fixing issues before the actual release.

I mean, I often C-1 patches that would be disruptive, as do many of the usual crowd. I'd encourage you to consider doing the same, out of respect for the time of colleagues and particularly the Security team when making last-minute back-ports.

The wider suggestion is an interesting idea. You should pitch it to RelEng, though I imagine they'd want to defer such work until after the GitLab migration rather than implementing such a tool from scratch twice within a year.

To clarify: I agree that we should not do backports shortly before a release. Deprecations and removals should not be backported at all. But doing a deprecation on master a week before or after the rc0 branch is cut seems fine to me. I'm not sure I understand the problem with that. Extensions authors still have a month to fix problems between rc0 and the actual release, right? Maybe I'm misunderstanding when and how extensions snapshots are made...

As to running CI for all extensions... it wouldn't be hard to write a script that pushes a DNM patch to every repo, right? And maybe even automatically file tickets if these patches fail to pass CI...

To clarify: I agree that we should not do backports shortly before a release. Deprecations and removals should not be backported at all. But doing a deprecation on master a week before or after the rc0 branch is cut seems fine to me. I'm not sure I understand the problem with that. Extensions authors still have a month to fix problems between rc0 and the actual release, right? Maybe I'm misunderstanding when and how extensions snapshots are made...

There are two sources of pain:

• People making big changes "just" after the branch is cut, before it's released. This pain mostly affects MediaWiki itself and deployed extensions.
• People making big changes "just" before the branch is cut. This pain mostly affects those extension developers who aren't pushing a patch every few hours.

Branches are cut from master at the same time or very shortly after the "final" alpha deployment branch is, for all ~1800 extensions and skins (not just the ~200 in Wikimedia production). After that, there's a period of manual testing for things that no-one cares about except in releases, like the installer, and a bunch of bugs are found and fixed. As with all Wikimedia development, those changes are made first to the master branch and then back-ported. We also backport major bug fixes that are discovered during the week of the deployment. The general guidance is that this post-cut process can take up to six weeks until rc.0 is cut, though in practice I'd not worry about it much beyond the first three–four. Naturally, LTS releases make for even more pressure to get things out of the door, and so rc.0 can unveil lots of bugs and need several follow-ups (as we found with the 1.35 release), but in general it's pretty free sailing after rc.0 is released.

For the first area of pain, large scale refactors or removals of code very frequently mean that these back-ports can't be easily made, or in some cases have to be re-written from scratch. In general the burden of this churn doesn't fall on the person that made the work by merging the refactor so swiftly, but on the bug fixer (or worse, someone like me just trying to help the release get out).

For the second area of pain, hard deprecations are, per long-standing development policy, effectively a breaking change for development (and for Wikimedia-deployed code), as they make CI fail. If one were e.g. to hard-deprecate the User class a week before REL1_37 were branched, hundreds of extensions would immediately be broken for development, and either their owners would find out a month or two later when they next pushed a patch, or (more frequently) helpers like me or tools like LibraryUpgrader would encounter it when fixing up the repo for other reasons.

I think this section should be split into statements about breaking changes and deprecations, and say different things about them, e.g.:

• Breaking changes and large or risky refactors SHOULD NOT be performed during the few weeks running up to a release being branched until the first release candidate is made, to make it easier to back-port bug fixes to release candidates.
• Hard deprecations SHOULD NOT be performed shortly before a release branch, to give extension authors time to fix any issues that may arise.

• Deprecations and removals MUST first take place on the master branch. Deprecations and removals SHOULD NOT be backported to release branches after the first release candidate is made.

Would that work?

Maybe I'm thinking about this wrong, but in my mind, the best time for extension developers to discover and fix issues is after the branch has been cut, so it's no longer a moving target. I assumed ExtensionDistributor will pick up any fixed made to the extension's snapshot branch. In my understanding, this would make sure that at the time of the release, the snapshot branches of extensions have also been fixed. To support this, CI should run against the extension when the branch is created.

As to not making major changes between the branching and the release of rc0: I think this is reasonable, but would need to be announced more clearly. Also, having at least a rough idea of the timing beforehand would allow teams to plan around this.