Page MenuHomePhabricator

MFA: Port mobile.watchstar and mobile.pagelist.scripts code to webpack
Closed, ResolvedPublic5 Story Points

Description

Code relating to the watchstar code currently lives in mobile.watchstar and mobile.pagelist.scripts. Beaming with the success of T207805, we will move it to webpack.

Acceptance criteria

  • There is no mobile.watchstar modules-these are now part of mobile.startup ! yay! (Thus: the watchstar code is loaded in the initial JS bundle for Minerva) and there are no deprecation warnings in the console
  • There is no mobile.pagelist.scripts modules -these are now part of mobile.startup ! yay! (Thus: the watchstar code is loaded in the initial JS bundle for Minerva)
  • Tests for mobile.pagelist.scripts and mobile.watchstar can be run in headless mode
  • References to M.require( 'mobile.pagelist.scripts/WatchstarPageList' ) have been removed in Mobilefrontend and there are no deprecation warnings in the console
  • Tests in tests/qunit/mobile.watchstar/ and tests/qunit/mobile.pagelist.scripts/ have been removed
  • resources/mobile.watchstar/watchstar.less has been relocated to resources/mobile.startup/watchstar.less to reflect which module it is tied to
  • Minerva is retained in a mergeable state throughout the migration.

Sign off steps

  • Progress test coverage is updated.
  • Make sure "Automate asset bundling" table is up to date in Progress
  • Make sure acceptance criteria all signed off.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterHygiene: move pageList.less to reflect src/mobile.startup
mediawiki/skins/MinervaNeue : mastermobile.watchstar is a deprecated module
mediawiki/extensions/MobileFrontend : masterWatchstar webpack migration follow ups
mediawiki/extensions/MobileFrontend : masterDrop redundant tests
mediawiki/extensions/MobileFrontend : masterMoving Watchstar, WatchstarGateway & WatchstarPagelist to webpack

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 7 2018, 12:52 AM
Jdlrobson renamed this task from Port mobile.watchstar and mobile.pagelist.scripts code to webpack to MFA: Port mobile.watchstar and mobile.pagelist.scripts code to webpack.Nov 15 2018, 6:59 PM
Jdlrobson triaged this task as High priority.
Restricted Application added a subscriber: Gilles. · View Herald TranscriptNov 20 2018, 3:48 PM
Jdlrobson set the point value for this task to 5.Nov 20 2018, 5:52 PM

Change 475766 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Moving Watchstar, WatchstarGateway & WatchstarPagelist to webpack

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

Jdlrobson added a subscriber: Jdrewniak.

Suspect that this will get merged today once T204876 is working locally for me...

Change 475766 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Moving Watchstar, WatchstarGateway & WatchstarPagelist to webpack

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

Jdlrobson removed a project: Patch-For-Review.

There are 3 patches needed to follow up this:

  1. references to mobile.watchstar in Minerva that will need to be updated to mobile.startup
  2. mobile.watchstar should be removed from MobileFrontend.
  3. tests/qunit/mobile.pagelist.scripts/WatchstarPageList.test.js references a deprecated module
Jdlrobson updated the task description. (Show Details)Nov 28 2018, 11:44 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson removed Jdlrobson as the assignee of this task.Nov 28 2018, 11:48 PM
Jdlrobson updated the task description. (Show Details)
Gilles removed a subscriber: Gilles.Nov 29 2018, 12:13 PM
Jdlrobson updated the task description. (Show Details)Nov 30 2018, 1:11 AM
Jdlrobson updated the task description. (Show Details)

Change 476793 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Drop redundant tests

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

Jdlrobson updated the task description. (Show Details)Nov 30 2018, 1:21 AM

Change 476793 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Drop redundant tests

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

Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Nov 30 2018, 11:24 PM

Change 476995 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] mobile.watchstar is a deprecated module

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

Change 476998 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Watchstar webpack migration follow ups

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

^ We should look to fix the deprecation warnings before the end of Monday as that's going to lead to a bunch of deprecation warnings in production😱.
Leaving in your capable hands!

Change 476998 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Watchstar webpack migration follow ups

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

Should the following be removed from extension.json?

		"mobile.watchstar": {
			"targets": [
				"mobile",
				"desktop"
			],
			"deprecated": true,
			"dependencies": [
				"mobile.startup"
			]
		}

I don't see one for the references module which had a similar port.

Change 476995 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] mobile.watchstar is a deprecated module

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

I think this should be moved too:

  • ./resources/mobile.startup/watchstar.less -> resources/mobile.startup/watchstar/

I'm not sure if this task covers watchlist:

  • resources/mobile.watchlist -> src/mobile.startup/watchlist
  • ./resources/mobile.special.watchlist.scripts -> src/mobile.startup/watchlist
  • ./tests/qunit/mobile.watchlist/ -> tests/node-qunit/mobile.startup/watchlist

Er, and I think this pagelist less?

  • ./resources/mobile.pagelist.styles => mobile.startup.styles/pagelist/

https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/477827/
took care of watchstar.less, thanks @Niedzielski

I believe mobile.watchlist/ is not part of this patch.

Change 478627 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Hygiene: move pageList.less to reflect src/mobile.startup

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

Er, and I think this pagelist less?
./resources/mobile.pagelist.styles => mobile.startup.styles/pagelist/

Nope this should stay where it is. It is used in addModuleStyles calls so should remain and be JS only:
ag T208921-followup x ~/git/core/extensions/MobileFrontend $ ag mobile.pagelist.styles
extension.json
85: "mobile.pagelist.styles": {
91: "resources/mobile.pagelist.styles/pagelist.less"
282: "mobile.pagelist.styles",

includes/specials/SpecialMobileContributions.php
64: 'mobile.pagelist.styles',

includes/specials/SpecialMobileEditWatchlist.php
223: 'mobile.pagelist.styles',

includes/specials/SpecialMobileHistory.php
112: 'mobile.pagelist.styles',

includes/specials/SpecialMobileWatchlist.php
50: 'mobile.pagelist.styles',

This looks done to me and I can sign off today.

Jdlrobson updated the task description. (Show Details)Dec 12 2018, 12:02 AM
Jdlrobson closed this task as Resolved.Dec 12 2018, 12:05 AM

Signing off. Folders are clear and wiki page updated.

Change 478627 abandoned by Jdlrobson:
Hygiene: move pageList.less to reflect src/mobile.startup

Reason:
Lemme know if I'm missing something!

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