Jump to content

Talk:Stable interface policy/Archive 1

About this board

ResourceLoader module names

3
Legoktm (talkcontribs)

Kinda related to the changes made as part of phab:T192623.

While the modules themselves are JavaScript/CSS/LESS and therefore unambiguously out of scope, the names of the modules themselves are often referenced in PHP code (e.g. $out->addModules( 'mediawiki.api' )) and in extension.json.

A strict reading of the "Scope" section would keep them out, but I'm leaning to thinking that we should consider them part of the PHP API, and explicitly document them as in scope of the policy.

Krinkle (talkcontribs)

I've been mostly following the policy already (open to interpretation), and would like it to be included as well.

Let's create a small RFC for this amendment? We'll need to determine a few details, such as how we define soft/hard deprecation.

DKinzler (WMF) (talkcontribs)
Reply to "ResourceLoader module names"
Summary by DKinzler (WMF)

This proposal has evolved into phab:T193613 and the Stable_Interface_Policy draft.

DKinzler (WMF) (talkcontribs)

Service objects are not intended to be instantiated directly. As such, their constructor signatures should not be considered part of the public stable interface by this policy. Ideally, extensions (and any application logic) only ever directly instantiates plain value objects. I propose to amend the deprecation policy to explicitly state that only constructor signatures of plain value objects are considered stable. Changes to the constructor signature of service objects and such do not have to follow the deprecation policy.

Krinkle (talkcontribs)

I agree they should be considered private. But, I think the policy already provides for this use case, with @internal. The constructors can be marked internal, right?

DKinzler (WMF) (talkcontribs)

We could do that, yes. But I'd prefer them to be considered internal per default. There are really just a few things that should be considered "newable" by application logic and extensions. I'd like to treat this the same as sub-classing: if you are directly calling the constructor that is not explicitly declared to be stable, you are on your own.

Simetrical (talkcontribs)

Service classes might be subclassed, though, like DefaultPreferencesFactory -> GlobalPreferencesFactory in extensions/GlobalPreferences/. Then they'll have to invoke the constructor, right?

Why don't we make the constructors not callable directly at all? PHP doesn't officially support friend functions, but you can do it by passing around callbacks. Make service class constructors private, then give each service class a method

  public static function provideConstructor( MediaWikiServices $services ) : callable {
    $services->takeConstructor( self::class, function ( ...$args ) {
      return new self( ...$args );
    };
  }

and then let MediaWikiServices keep an array of constructors it can call but no one else has access to. This might be, um, slightly hacky, but there's nothing else that's realistically going to prevent everyone in the codebase (including core!) from directly instantiating all these classes.

DKinzler (WMF) (talkcontribs)

We already consider subclassing unsupported, unless the class or interface explicitly documents that it can be subclassed/implemented by extensions. It's a bit buried, and doesn't talk about constructors, but it's there:

some classes expect to be subclassed in extensions; in those cases protected functions also are included in the API. These classes should have a note in their documentation comment that they expect subclassing. If no note is present, it SHOULD be assumed that the class is not expected to be subclassed.

Perhaps we should have a section that explicitly discusses subclassing and constructors.


That being said: if DefaultPreferencesFactory expects to be subclassed, its constructor should be considered a stable public interface which is subject to the deprecation policy.

DKinzler (WMF) (talkcontribs)

In order to address some issues revolving around subclassing and implementing interfaces in extensions, I propose to amend this policy, see phab:T193613. The core of the proposal is to move the definition of the stable interface out of the Scope section into a separate policy, drafted at Stable_Interface_Policy.

Reply to "Proposal to Amend"

How does this policy affect unused code?

2
X-Savitar (talkcontribs)

I've seen people ask about this and I've also encountered it myself. How does this policy address unused code? Like code that was introduced but never used ranging from variables, constants, classes, etc. Maybe it's worth mentioning in the policy how this should be treated?


References (from #mediawiki-core):

22:55 (dmaza) do we have a policy about removing un-used code? Does it have to be soft deprecated/hard deprecated or can it be straight up removed from the code base?
Krinkle (talkcontribs)

The policy does not mention unused code, because it is not treated differently.

All public interfaces to the MediaWiki core PHP code that was released, must follow the deprecation policy prior to removal.

Local variables are not public, and do not follow the deprecation policy as such.

There is no exemption for "unused code". There are multiple reasons for this, but for me the main one is that we can't really know whether something is unused.

Wikimedia Foundation has all its code public in our Gerrit, and with pretty good search. Although even with that search, we've definitely missed things in the past. It's not perfect. It's always better to deprecate first and allow ourselves to find warnings from production.

Apart from WMF, it's important to remember that MediaWiki is an open-source project used for many other websites. One would need to know every website in the world and every possible way to publish source code relating to MediaWiki, and search it. One would also need to know which version of all those projects are currently in deployment or not. This isn't realistic.

Besides public code, there is also site configuration which is usually not made public (Wikipedia represents the exception there, not the norm).

Class alias deprecation and removal

3
Reedy (talkcontribs)

So, we add class aliases when rename stuff... What "deprecation" policy should we be using to remove them? Remove the next major release (especially if we've cleaned up usages in core/extensions?)

I note we don't add since or deprecated tags to the calls, so it's harder to track them... Maybe we should start doing that, rather than having to git blame for them all

Legoktm (talkcontribs)

We should be following the standard deprecation policy, with the caveat that it might not be technically possible to hard deprecate (like class members, constants, etc.).

We should definitely add @since and @deprecated tags to the class alias calls, and ensure that it's mentioned in the release notes.

Reedy (talkcontribs)

https://phabricator.wikimedia.org/T195576 already filed for backfilling

Hard deprecation would be nice, but isn't always necessary. If we're not at least using deprecated tags, we're doing ourselves and other people a dis-service

Summary by Tgr (WMF)

The RfC has been accepted and this page updated accordingly.

RobLa-WMF (talkcontribs)
There are no older topics