Page MenuHomePhabricator

Page preview source hygiene
Closed, ResolvedPublic

Description

  • Move ui sources to ui/ folder
  • Update mw-node-qunit to version 2.1.1
    • 2.1.0 had a bug when erroring because of using console.err
  • Use standard ES modules instead of common.js

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 28 2017, 1:04 PM
Jhernandez updated the task description. (Show Details)Jul 28 2017, 1:05 PM
Jhernandez renamed this task from Page preview hygiene to Page preview source hygiene.Jul 28 2017, 1:32 PM
Jhernandez updated the task description. (Show Details)

Change 368419 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Hygiene: Move ui renderer.js to ui/ folder

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

Change 368420 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Deps: Upgrade mw-node-qunit

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

Change 368425 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Hygiene: Move settingsDialog UI code to ui/

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

Change 368460 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Use EcmaScript modules instead of common.js modules

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

Jhernandez triaged this task as Normal priority.Jul 31 2017, 11:03 AM
Jhernandez updated the task description. (Show Details)
Jhernandez moved this task from To Triage to 2017-18 Q1 on the Readers-Web-Backlog board.

Moving to code review for eyes.

Change 368419 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: Move ui renderer.js to ui/ folder

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

Change 368420 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Deps: Upgrade mw-node-qunit

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

Change 368425 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: Move settingsDialog UI code to ui/

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

It may be interesting to do a session to share how the module spec works and how it is made work on popups once we've merged it.

I sent an email about this. Moved to blocked until we get some replies and a knowledge share has happened if necessary.

This patch is the best example of how common.js relates to ES modules in a real world example. By reviewing it you see how they relate.

I'm happy to walk through the changes, but I'd also encourage team mates to read these at some point:

This is standard javascript we are talking about, standardized on ES2015. So it is important to learn about it.

We've done a session explaining the module system, why we should use it, the migration, etc.

If there are any salient points please ask and I'll put them in next week's sync.

Meanwhile, I'd appreciate review.

I plan to merge at end of day provided there have been no objections. I'd rather not get stuck in rebase hell!

Change 368460 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Use EcmaScript modules instead of common.js modules

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

phuedx closed this task as Resolved.Aug 8 2017, 11:26 AM

🎉