Hygiene: Don't use .done, use promises A/A+
Closed, ResolvedPublic
Actions

Description

Remove usages of .done/.fail/.always and use promises A/A+ instead (.then/.catch)

Noticed when signing off T170812: Return promises from QUnit tests

Details

Related Gerrit Patches:

mediawiki/extensions/Popups : master	Hygiene: Don't rely on .fail, use Promises/A+
mediawiki/extensions/Popups : master	Hygiene: Use promises A/A+ everywhere

Related Objects

Mentioned Here: T170812: Return promises from QUnit tests

Event Timeline

Jhernandez created this task.Aug 22 2017, 11:27 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 22 2017, 11:27 AM

Jhernandez added a project: Unplanned-Sprint-Work.Aug 22 2017, 11:28 AM

Jhernandez moved this task from Incoming to 2017-18 Q1 on the Readers-Web-Backlog board.

Jhernandez moved this task from To Do to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.

Change 373054 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Hygiene: Use promises A/A everywhere

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

gerritbot added a project: Patch-For-Review.Aug 22 2017, 11:36 AM

Change 373054 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: Use promises A/A everywhere

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

ReleaseTaggerBot added a project: MW-1.30-release-notes (WMF-deploy-2017-08-22 (1.30.0-wmf.15)).Aug 22 2017, 3:00 PM

There's one usage of fail in src/actions.js
Should that be a catch?

Jdlrobson removed a project: Patch-For-Review.Aug 22 2017, 6:41 PM

@Jdlrobson I saw that but .fail is not compatible with .catch straight forward. Specially when chaining promises like on this case where after that there is a $.when with to promises.

We need to be sure the semantics of the change are the same when we migrate that line to .catch.

You're right tho, that is part of this task... I'll see if I can work it out.

Jhernandez claimed this task.Aug 23 2017, 4:55 PM

Change 373327 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Hygiene: Don't rely on .fail, use Promises/A

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

gerritbot added a project: Patch-For-Review.Aug 23 2017, 5:47 PM

Jdlrobson moved this task from Needs More Work to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Aug 23 2017, 5:54 PM

Change 373327 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: Don't rely on .fail, use Promises/A

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