Page MenuHomePhabricator

deprecate Language::viewPrevNext
Closed, ResolvedPublic3 Story Points

Description

Follow up of discussion in WMTC 2018 we think we should sunset next/prev functions from language class, to remove dependencies on Message (cyclic) and Title (spurious).

  • soft deprecate and remove usages from core (1.33)
  • Remove usage from extensions (https://codesearch.wmflabs.org/search/?q=viewPrevNext&i=nope):
    • wikiworksdev / CloneDiff
    • extensions / Wikibase
    • extensions / WhosOnline
    • extensions / SpamRegex
    • extensions / RegexBlock
    • extensions / CloneDiff
    • SemanticMediaWiki / SemanticMediaWiki
  • hard deprecate (1.34)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
eranroz updated the task description. (Show Details)Oct 25 2018, 8:12 PM

My comment with the Deprecation policy link was simply because we were discussing the possibility of an external contractor working on this task, and the link seemed relevant to have available. Sorry, I should have included context.

My comment with the Deprecation policy link was simply because we were discussing the possibility of an external contractor working on this task, and the link seemed relevant to have available. Sorry, I should have included context.

I'm volunteer editor not paid external contractor :-)
Anyway thanks for the comment - whoever change it (WMF engineer/ volunteer/3rd party etc) should follow the depreciation policy so don't hesitate to raise any issue if you have concern that the commit doesn't follow it.

Change 469667 had a related patch set uploaded (by Daniel Kinzler; owner: Eranroz):
[mediawiki/core@master] Sunsetting viewPrevNext

https://gerrit.wikimedia.org/r/469667

Change 469667 merged by jenkins-bot:
[mediawiki/core@master] Sunsetting viewPrevNext

https://gerrit.wikimedia.org/r/469667

daniel closed this task as Resolved.May 13 2019, 3:30 PM
daniel reopened this task as Open.
daniel claimed this task.

Re-opening, extensions still need to be fixed.

CCicalese_WMF triaged this task as Low priority.May 13 2019, 3:34 PM
Aklapper updated the task description. (Show Details)May 14 2019, 2:22 PM

Re-opening, extensions still need to be fixed.

In my ideal world that would be dedicated subtasks... Adding MediaWiki-extensions-General for the time being and listed the extensions in the task desc.

Change 512466 had a related patch set uploaded (by Eranroz; owner: Eranroz):
[mediawiki/extensions/Wikibase@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512466

Change 512467 had a related patch set uploaded (by Eranroz; owner: Eranroz):
[mediawiki/extensions/CloneDiff@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512467

Change 512468 had a related patch set uploaded (by Eranroz; owner: Eranroz):
[mediawiki/extensions/RegexBlock@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512468

Change 512469 had a related patch set uploaded (by Eranroz; owner: Eranroz):
[mediawiki/extensions/SpamRegex@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512469

Change 512470 had a related patch set uploaded (by Eranroz; owner: Eranroz):
[mediawiki/extensions/WhosOnline@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512470

Change 512470 abandoned by Eranroz:
Using new buildPrevNextNavigation

Reason:
non obvious change (requires changes to pager and special page)

https://gerrit.wikimedia.org/r/512470

WDoranWMF set the point value for this task to 3.
WDoranWMF added a subscriber: WDoranWMF.

This task may need to be broken down but could simply be not as trivial as it looks.

A new wrinkle to this became evident in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WhosOnline/+/512470 (now abandoned for this reason): Extensions that implement the Pager interface may want to use buildPrevNextNavigation() to implement the getNavigationBar method. But buildPrevNextNavigation() is now protected in the SpecialPage base class, so Pagers can't use it.

It looks like the code in buildPrevNextNavigation() will have to be factored out into a standalone helper class, or into a trait, so it can be used in both SpecialPages and Pagers.

Pinging @Clarakosi to have a look.

Change 513240 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/core@master] Refactor buildPrevNextNavigation

https://gerrit.wikimedia.org/r/513240

Change 513240 merged by jenkins-bot:
[mediawiki/core@master] Refactor buildPrevNextNavigation

https://gerrit.wikimedia.org/r/513240

Change 512470 restored by Eevans:
Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512470

Change 514731 had a related patch set uploaded (by Clarakosi; owner: Eranroz):
[mediawiki/extensions/WhosOnline@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/514731

Change 516655 had a related patch set uploaded (by Clarakosi; owner: Eranroz):
[mediawiki/extensions/SpamRegex@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/516655

Change 516678 had a related patch set uploaded (by Clarakosi; owner: Eranroz):
[mediawiki/extensions/RegexBlock@master] [wip]Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/516678

Change 512466 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512466

Change 512467 merged by jenkins-bot:
[mediawiki/extensions/CloneDiff@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512467

Change 516678 merged by jenkins-bot:
[mediawiki/extensions/RegexBlock@master] [wip]Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/516678

Change 516655 merged by jenkins-bot:
[mediawiki/extensions/SpamRegex@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/516655

Change 514731 merged by jenkins-bot:
[mediawiki/extensions/WhosOnline@master] Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/514731

Clarakosi updated the task description. (Show Details)Jun 13 2019, 1:23 PM

@CCicalese_WMF Any ideas on who could do the SMW patch? I mean, the fix itself is trivial (change MessageBuilder::prevNextToText to do the same thing as IndexPage::buildPrevNextNavigation now does), but fixing the test needs a little more work, and running the test means I'd have to install SMW :)

@CCicalese_WMF Any ideas on who could do the SMW patch? I mean, the fix itself is trivial (change MessageBuilder::prevNextToText to do the same thing as IndexPage::buildPrevNextNavigation now does), but fixing the test needs a little more work, and running the test means I'd have to install SMW :)

@Kghbln might be able to help on that front, or at least point to someone who can. =)

Change 512469 abandoned by Daniel Kinzler:
Using new buildPrevNextNavigation

Reason:
Looks like this was done in I912c7ab80dd94a16e22f14be03b013f97b1bfa57

https://gerrit.wikimedia.org/r/512469

@daniel and I discussed that I would assist @Clarakosi in submitting a pull request including tests to https://github.com/SemanticMediaWiki/SemanticMediaWiki.

@Dinoguy1000 Thanks for pinging. I have created tracking issue 4092 for this.

@daniel and I discussed that I would assist @Clarakosi in submitting a pull request including tests to https://github.com/SemanticMediaWiki/SemanticMediaWiki.

@CCicalese_WMF Thanks a lot for your engagement here. Much appreciated, indeed!

Change 512468 abandoned by Daniel Kinzler:
Using new buildPrevNextNavigation

https://gerrit.wikimedia.org/r/512468

daniel renamed this task from remove prev/next message dependency from Language to deprecate Language::viewPrevNext.Fri, Jun 28, 2:10 PM
daniel updated the task description. (Show Details)
daniel updated the task description. (Show Details)Fri, Jun 28, 2:14 PM

The pull request for SMW is up, so the final step from our side would be to hard-deprecate Language::viewPrevNext.

@Kghbln can we go ahead with that?

Not just that. CI is also complaining about use of deprecated DB_SLAVE and other deprecated things. SMW seems incompatible with the master branch of MW.

I'm a bit confused. SMW doesn't claim to be compatible with MW master. It's of course still useful to see in which ways it is incompatible, but to me it looks like that now a single patch would need to fix all incompatibilities to turn CI green, so other things can then be merged again. Is this the case? That seems awkward.

Overall, I don't think we need to wait for the change to me merged into SMW, we can go for hard deprecation in MW core now. This won't make SMW's incompatibility 1.34 any worse than it already is.

Clarakosi updated the task description. (Show Details)Sat, Jun 29, 1:05 PM

SMW PR has been merged and with that, all extensions are updated.

@Clarakosi: Excellent! Can you add the wfDeprecated() line to the old method as well? That's the last thing left to do here, I believe.

Change 519757 had a related patch set uploaded (by Clarakosi; owner: Clarakosi):
[mediawiki/core@master] Hard deprecrate Language::viewPrevNext()

https://gerrit.wikimedia.org/r/519757

Change 519757 merged by jenkins-bot:
[mediawiki/core@master] Hard deprecrate Language::viewPrevNext()

https://gerrit.wikimedia.org/r/519757

daniel closed this task as Resolved.Mon, Jul 1, 10:05 AM
daniel updated the task description. (Show Details)
daniel updated the task description. (Show Details)
daniel updated the task description. (Show Details)