Page MenuHomePhabricator
Create Task
Maniphest T138788

Refactor Wikibase config variables used by MobileFrontend
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To
• Jhernandez
Authored By
• jhobs
Jun 27 2016, 9:17 PM
Tags
Referenced Files
None
Subscribers
Aklapper
• bmansurov
gerritbot
Jdlrobson
• Jhernandez
• jhobs
phuedx
Zppix

Description

Spawned off of T138738 to reduce confusion around the meaning of wikibase-related config variables.

AC

  • wgMFUseWikibaseDescription -> wgMFUseWikibase
  • Fold wgMFDisplayWikibaseDescription and wgMFDisplayWikibaseDescriptionsAsTaglines into new wgMFDisplayWikibaseDescriptions, changing it from a boolean to an object describing which features on which to display descriptions
  • Deprecation of wgMFUseWikibaseDescription and wgMFDisplayWikibaseDescription

[ ] Corresponding variables changed in mediawiki-config as well (To be done in follow-up task when variables are fully deprecated)

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Add extendSearchParams helper functionmediawiki/extensions/MobileFrontendmaster+197 -43
Further split $wgMFDisplayWikibaseDescriptionsmediawiki/extensions/MobileFrontendmaster+11 -3
Use wgMFDisplayWikibaseDescriptions in clientmediawiki/extensions/MobileFrontendmaster+30 -15
Extend MobileContext#shouldShowWikibaseDescriptionsmediawiki/extensions/MobileFrontendmaster+29 -8
Add wgMFDisplayWikibaseDescriptions to RL configmediawiki/extensions/MobileFrontendmaster+19 -0
Deprecate $wgMFDisplayWikibaseDescriptionmediawiki/extensions/MobileFrontendmaster+119 -41
Deprecate $wgMFUseWikibaseDescriptionmediawiki/extensions/MobileFrontendmaster+37 -6
Add MobileContext#shouldShowWikibaseDescriptionsmediawiki/extensions/MobileFrontendmaster+56 -8
Refactor Wikibase config variables to be less confusingmediawiki/extensions/MobileFrontendmaster+426 -46
Customize query in gerrit

Related Objects
Search...

Event Timeline

• jhobs created this task.Jun 27 2016, 9:17 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 27 2016, 9:17 PM
• jhobs mentioned this in T138738: [Regression] Wikidata description not showing in search results on mobile web.Jun 27 2016, 9:18 PM
gerritbot subscribed.Jun 27 2016, 9:33 PM

Change 296342 had a related patch set uploaded (by Jhobs):
Refactor wikibase config variables to be less confusing

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

gerritbot added a project: Patch-For-Review.Jun 27 2016, 9:33 PM
• jhobs added projects: Reading-Web-Sprint-75-Quantitative-Paralysis, Unplanned-Sprint-Work.Jun 27 2016, 9:39 PM
Jdlrobson moved this task from Needs Analysis to To Do on the Reading-Web-Sprint-75-Quantitative-Paralysis board.Jun 27 2016, 10:01 PM
Jdlrobson moved this task from To Do to Doing on the Reading-Web-Sprint-75-Quantitative-Paralysis board.
MaxSem moved this task from Unsorted to Needs refactor on the Technical-Debt board.Jun 28 2016, 5:34 AM
Jdlrobson triaged this task as Medium priority.Jun 28 2016, 5:10 PM
• jhobs added a project: Web-Team-Backlog-Archived.Jun 28 2016, 5:12 PM
• jhobs moved this task from Incoming to 2015-16 Q4 on the Web-Team-Backlog-Archived board.
• jhobs added a subscriber: phuedx.Jun 28 2016, 5:25 PM

Those names are still shit. After discussing with @phuedx and @Jdlrobson, I'm gonna try a few different ways and see what sticks, then update the task accordingly.

• jhobs moved this task from Doing to Code Review on the Reading-Web-Sprint-75-Quantitative-Paralysis board.Jul 1 2016, 11:08 PM
• jhobs updated the task description. (Show Details)Jul 1 2016, 11:49 PM
• jhobs set the point value for this task to 3.
• jhobs removed • jhobs as the assignee of this task.Jul 2 2016, 1:31 AM

Freeing this up as I'll be out of town next week.

phuedx claimed this task.Jul 4 2016, 8:36 AM
phuedx moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-75-Quantitative-Paralysis board.
phuedx moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-75-Quantitative-Paralysis board.Jul 5 2016, 1:32 PM

^ Fixing a failing unit test case shouldn't block an initial review…

• bmansurov moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-75-Quantitative-Paralysis board.Jul 5 2016, 3:05 PM
• bmansurov added a project: Reading-Web-Sprint-76-Retrosprekfast.Jul 5 2016, 4:08 PM
• bmansurov moved this task from Needs Analysis to -1 (Needs More Work) on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 5 2016, 4:16 PM
MBinder_WMF removed a project: Unplanned-Sprint-Work.Jul 5 2016, 5:01 PM
phuedx moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 6 2016, 5:04 PM
phuedx added a subscriber: • bmansurov.

@bmansurov: I've tried to address your last (largest) comment in the latest patch set. Thanks for the feedback.

Jdlrobson moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 6 2016, 10:01 PM
Liuxinyu970226 subscribed.Jul 6 2016, 11:20 PM
MBinder_WMF moved this task from 2015-16 Q4 to 2016-17 Q1 on the Web-Team-Backlog-Archived board.Jul 7 2016, 8:11 PM
gerritbot added a comment.Jul 11 2016, 10:39 AM

Change 298261 had a related patch set uploaded (by Phuedx):
Add MobileContext#shouldShowWikibaseDescriptions

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

gerritbot added a comment.Jul 11 2016, 10:39 AM

Change 298262 had a related patch set uploaded (by Phuedx):
Extend MobileContext#shouldShowWikibaseDescriptions

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

gerritbot added a comment.Jul 11 2016, 10:39 AM

Change 298263 had a related patch set uploaded (by Phuedx):
Deprecate $wgMFUseWikibaseDescription

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

phuedx added a comment.Jul 11 2016, 10:40 AM

There are two more changes to add to this sequence: the introduction of $wgMFDisplayWikibaseDescriptions and the extendSearchParams function, both of which I'm extracting currently…

gerritbot added a comment.Jul 12 2016, 8:56 AM

Change 298426 had a related patch set uploaded (by Phuedx):
Deprecate $wgMFDisplayWikibaseDescription

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

gerritbot added a comment.Jul 12 2016, 9:18 AM

Change 298430 had a related patch set uploaded (by Phuedx):
Add wgMFDisplayWikibaseDescriptions to RL config

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

gerritbot added a comment.Jul 12 2016, 9:42 AM

Change 298432 had a related patch set uploaded (by Phuedx):
Use wgMFDisplayWikibaseDescriptions in client

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

gerritbot added a comment.Jul 12 2016, 9:42 AM

Change 298433 had a related patch set uploaded (by Phuedx):
Further split $wgMFDisplayWikibaseDescriptions

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

gerritbot added a comment.Jul 12 2016, 9:50 AM

Change 298434 had a related patch set uploaded (by Phuedx):
Add extendSearchParams helper function

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

phuedx moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 12 2016, 9:56 AM
Jdlrobson moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 12 2016, 8:34 PM

There are a few open questions on the patchsets (see +1s and -1s) but nothing too major. I hope we can wrap this work up Wednesday - we may want to hangout after standup to do so.

gerritbot added a comment.Jul 12 2016, 9:31 PM

Change 296342 abandoned by Jdlrobson:
Refactor Wikibase config variables to be less confusing

Reason:
This has now been split up into https://gerrit.wikimedia.org/r/298262 and friends - let's carry conversation over there.

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

phuedx moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 13 2016, 9:02 AM
gerritbot added a comment.Jul 13 2016, 2:41 PM

Change 298261 merged by jenkins-bot:
Add MobileContext#shouldShowWikibaseDescriptions

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

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-07-12_(1.28.0-wmf.10)).Jul 13 2016, 3:00 PM
phuedx moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 13 2016, 5:12 PM
phuedx moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 14 2016, 8:42 AM

@Jdlrobson: I've responded to your comment. Perhaps we can nip this in the bud immediately before/after standup.

gerritbot added a comment.Jul 14 2016, 1:29 PM

Change 298262 merged by jenkins-bot:
Extend MobileContext#shouldShowWikibaseDescriptions

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

gerritbot added a comment.Jul 14 2016, 1:30 PM

Change 298263 merged by jenkins-bot:
Deprecate $wgMFUseWikibaseDescription

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

gerritbot added a comment.Jul 14 2016, 1:42 PM

Change 298426 merged by jenkins-bot:
Deprecate $wgMFDisplayWikibaseDescription

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

gerritbot added a comment.Jul 14 2016, 1:52 PM

Change 298430 merged by jenkins-bot:
Add wgMFDisplayWikibaseDescriptions to RL config

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

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-07-19_(1.28.0-wmf.11)).Jul 14 2016, 2:00 PM
Jdlrobson moved this task from Code Review to Ready for Signoff on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 14 2016, 5:18 PM
Jdlrobson added a comment.Jul 14 2016, 5:20 PM

I'm going to carefully go over the Wikimedia config today to ensure this doesn't interrupt service on any live wikis given this discussion. Expect subtasks! :)

gerritbot added a comment.Jul 14 2016, 5:23 PM

Change 298432 merged by jenkins-bot:
Use wgMFDisplayWikibaseDescriptions in client

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

Jdlrobson added a comment.Jul 14 2016, 7:05 PM

So I had a super close look at this.
We only introduced MFDisplayWikibaseDescriptionsAsTaglines June 27th. Before this change enabling $wgMFUseWikibaseDescription would control the Wikidata description tagline. We've now deprecated this config variable 2 weeks later in favour of $wgMFDisplayWikibaseDescriptions = [ 'tagline' => true ].

The result of this is any site using the sole config variable $wgMFUseWikibaseDescription = true from before June 27th to show taglines who upgrades will now lose their tagline unless they turn on $wgMFDisplayWikibaseDescriptions immediately.

This is all very confusing and I doubt many people are using Wikibase for this purpose so I think we should just accept this is the case.

Another engineer should feel free to sign off knowing the above BUT they should open a new task for next sprint to remove all the deprecated config variables as soon as these changes are live everywhere (from 21st July 2016).

• jhobs added a comment.EditedJul 14 2016, 7:10 PM
In T138788#2463523, @Jdlrobson wrote:

So I had a super close look at this.
We only introduced MFDisplayWikibaseDescriptionsAsTaglines June 27th. Before this change enabling $wgMFUseWikibaseDescription would control the Wikidata description tagline. We've now deprecated this config variable 2 weeks later in favour of $wgMFDisplayWikibaseDescriptions = [ 'tagline' => true ].

Just for context: This was because it was expected that there would be a slight overhaul on these config variables shortly after introducing $wgMFDisplayWikibaseDescriptionsAsTaglines, but it was not expected that the most appropriate solution would be to use $wgMFDisplayWikibaseDescriptions as an object (although it was known to be a potential solution). Thus, we didn't initially think that we'd be introducing the taglines variable and then immediately removing it. It took more investigation to determine that was the path forward. I think the slight confusion here in terms of refactoring a new variable immediately is understandable and permissible given that we're doing it in such a close timeframe and it was announced in the original task that a follow-up refactoring would be done.

• jhobs added a comment.Jul 14 2016, 7:13 PM
In T138788#2463523, @Jdlrobson wrote:

Another engineer should feel free to sign off knowing the above BUT they should open a new task for next sprint to remove all the deprecated config variables as soon as these changes are live everywhere (from 21st July 2016).

As someone who contributed a fair amount to this task, I'd rather leave this to someone with fresher eyes.

Jdlrobson added a subscriber: • Jhernandez.Jul 14 2016, 9:49 PM

I nominate @Jhernandez given his distance from the task for sign off.

phuedx reassigned this task from phuedx to • Jhernandez.Jul 15 2016, 8:59 AM

Per T138788#2464258.

phuedx added a comment.Jul 15 2016, 10:46 AM

@Jdlrobson: 298434: Add extendSearchParams helper function and its dependency need to re-reviewed as they didn't get merged for one reason or another…

phuedx moved this task from Ready for Signoff to Code Review on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 15 2016, 10:46 AM
gerritbot added a comment.Jul 15 2016, 3:39 PM

Change 298433 merged by jenkins-bot:
Further split $wgMFDisplayWikibaseDescriptions

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

gerritbot added a comment.Jul 15 2016, 3:41 PM

Change 298434 merged by jenkins-bot:
Add extendSearchParams helper function

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

Jdlrobson moved this task from Code Review to Ready for Signoff on the Reading-Web-Sprint-76-Retrosprekfast board.Jul 15 2016, 4:55 PM
• Jhernandez mentioned this in T140600: Remove deprecated wikibase config variables from configurations.Jul 18 2016, 10:51 AM
• Jhernandez added a parent task: T140600: Remove deprecated wikibase config variables from configurations.
• Jhernandez closed this task as Resolved.Jul 18 2016, 11:12 AM

I've updated https://www.mediawiki.org/wiki/Extension:MobileFrontend docs.

Please don't forget to update the docs.

Seems done

Liuxinyu970226 unsubscribed.Jul 19 2016, 5:05 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits