Page MenuHomePhabricator

Decide and document backwards compatibility policy for PageForms
Closed, ResolvedPublic

Description

Basically see my comment at T228049#5333127. It claims to support MW 1.27 (which is now dead), has a security review that's still open and reports several issues (T149869), and contains trivial errors such as undefined variables.
At the very least, it should be updated to support newer MW. However, no compatibility policy is set on mw.org, and thus I'm unsure what version to require. If we want the 'master' policy, I believe we should bump the requirement to the older version currently alive, which is 1.31. Otherwise, just bump it to 1.34. Then we can start with fixing obvious errors etc.

CC @Yaron_Koren as author.

Event Timeline

@Daimona: I do not understand why bumping requirements changes anything or why that is even mentioned. You could also start fixing errors without bumping?

@Aklapper Yeah, I'm sorry, I only quickly mentioned it in T228049#5333127. This extension uses methods removed from core several versions ago (like ApiBase::dieUsage or OutputPage::addWikitext - T228048). Depending on the compat policy, we should either add back-compat code or just replace them. Such methods seem to cause several issues reported by phan in https://integration.wikimedia.org/ci/job/mwext-php72-phan-docker/3431/console, that's why I first wanted to know what to do with them.
That said, I definitely can start fixing unrelated stuff, and no doubt I'll find something to fix.

Is this a task, or what?

I didn't catch that. However, if you mean what's to do here, I was asking for what the compatibility policy for PageForms should be, and what's the minimum MW version it should support. This way I can properly fix whatever was reported in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/PageForms/+/523162/ (where, of note, many issues are due to SMW missing in the container, but I don't know how and if we can install it).

Would it make sense, then, to rename this to something like "Add phan to Page Forms"?

@Daimona: You may want to edit this task and change the task summary to "Clarify compatibility policy / minimum supported MW version" instead of "hugely out of date" which is not really helpful or constructive to allow realizing what's requested. Or what Yaron proposed. :)

Nikerabbit renamed this task from PageForms is hugely out of date to Decide and document backwards compatibility policy for PageForms.Jul 16 2019, 6:43 AM

Phan is not the only thing that is hampered by lack of clear BC policy. As I wrote in T228049#5335935, the code is using a lot of syntax that no other extension is using anymore, because it claims to still support PHP5. It also makes other developers avoid touching the code, both because they don't know what is expected and because it is so different from other extensions.

Yeah, I'm sorry, I sort of changed my mind while writing the description and didn't update the title accordingly. Nevertheless, what I wrote in the description is what this task should be about.

Would it make sense, then, to rename this to something like "Add phan to Page Forms"?

I'd keep it in a separate task, which would then be a parent task of this one.

@Daimona: You may want to edit this task and change the task summary to "Clarify compatibility policy / minimum supported MW version" instead of "hugely out of date" which is not really helpful or constructive to allow realizing what's requested. Or what Yaron proposed. :)

Indeed, per above. Niklas chose the title I'd have chosen myself.

Phan is not the only thing that is hampered by lack of clear BC policy. As I wrote in T228049#5335935, the code is using a lot of syntax that no other extension is using anymore, because it claims to still support PHP5. It also makes other developers avoid touching the code, both because they don't know what is expected and because it is so different from other extensions.

Exactly.

Hi - I don't know if this is a good title for this task either, since, whatever you think of Page Forms' current compatibility policy, it is clearly stated on the extension's main wiki page, where it says that "MW 1.23+" is supported:

https://www.mediawiki.org/wiki/Extension:Page_Forms

That is indeed the compatibility policy. Now, you could argue that it's an unreasonable policy, especially because it means that PHP 5.3, 5.4 and 5.5 all have to be supported, which in turn means that syntax like "$myArray = []" can't be used. Not to mention that MW 1.23 came out over 5 years ago. But that's a separate discussion. It seems to me that this task should either be renamed, or abandoned... unless there's some other issue here.

Hi - I don't know if this is a good title for this task either, since, whatever you think of Page Forms' current compatibility policy, it is clearly stated on the extension's main wiki page, where it says that "MW 1.23+" is supported:

That's the supported MW version, but I was talking about this policy. It's usually added to the {{extension}} template, and I didn't find it for PageForms. Although it's possible that it's written elsewhere and I've missed it.
Also, note that mw.org claims 1.23+ as you said, but extension.json reads 1.27+. I'd lean towards 1.27 being the right choice here (just because code docs are usually more updated than on-wiki docs), but it's only important to have the same version in both places, whatever it is.

Now, you could argue that it's an unreasonable policy, especially because it means that PHP 5.3, 5.4 and 5.5 all have to be supported, which in turn means that syntax like "$myArray = []" can't be used. Not to mention that MW 1.23 came out over 5 years ago. But that's a separate discussion.

Agreed that this is out of the scope of this task. Yes, such a policy could be hard to maintain, costs may overwhelm benefits etc., but that's not something urgent. Knowing exactly what the minimum supported MW version is, and consequently how to fix some of the errors found by phan, is enough for this task.

And lastly, I guess based on your comments that the BC policy should be "master", supporting either MW 1.23+ or 1.27+, right?

Oh, okay - I knew about that "compatibility policy" parameter, but never went through all of my extensions' pages to make sure it's there. I just added "compatibility policy=master" to the Page Forms extension's main page. (The compatibility policy is "master" for all of my extensions.)

There's actually no conflict with what's in extension.json - the "1.27+" there means that using extension.json, i.e. wfLoadExtension(), is only guaranteed to work with MW 1.27 or higher; for earlier versions, require_once() or similar should be used instead.

Daimona assigned this task to Yaron_Koren.

Oh, okay - I knew about that "compatibility policy" parameter, but never went through all of my extensions' pages to make sure it's there. I just added "compatibility policy=master" to the Page Forms extension's main page. (The compatibility policy is "master" for all of my extensions.)

Great, thanks!

There's actually no conflict with what's in extension.json - the "1.27+" there means that using extension.json, i.e. wfLoadExtension(), is only guaranteed to work with MW 1.27 or higher; for earlier versions, require_once() or similar should be used instead.

Ah right, that totally makes sense.

I believe this task is now resolved, and I'll try to keep MW 1.23 compat when touching the extension.