Last modified: 2012-11-29 12:38:33 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T39982, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 37982 - Use .done() instead of .then() when adding success-handlers on Promise objects
Use .done() instead of .then() when adding success-handlers on Promise objects
Status: VERIFIED FIXED
Product: MediaWiki extensions
Classification: Unclassified
WikidataRepo (Other open bugs)
unspecified
All All
: Low trivial (vote)
: ---
Assigned To: Wikidata bugs
https://gerrit.wikimedia.org/r/gitweb...
: javascript, need-volunteer
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-27 08:27 UTC by Krinkle
Modified: 2012-11-29 12:38 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Krinkle 2012-06-27 08:27:24 UTC
Hi,

I didn't know where else to put it so I'll write it here. When randomly looking through a diff on Gerrit I noticed a few mentions of this the following (it wasn't changed in that commit, but shown in the diff context):

---
»       this.performApiAction( action )
»       »       .then( degrade );
---
»       var deferred = this.performApiAction( this.API_ACTION.SAVE )
»       »       .then( $.proxy( ... ) );
---
etc.

.then is literally just this:
> then: function( doneFns, failFns, progressfns ) {
>     deferred.done( doneFns ).fail( failFns ).progress( progressfns );
>     return this;
> },

So just use .done(). Even when binding both 'done' and 'fail' at the same time, I'd still recommend using them directly for clarity, but that's just me :)
Comment 1 Daniel A. R. Werner 2012-08-02 14:44:01 UTC
I agree, especially using .then() with two arguments is not very readable. It's pretty easy to mistake then for done or the other way around.
Comment 2 Daniel Friesen 2012-08-03 01:43:19 UTC
Note that es-harmony seems to be standardizing promises in such a way they'll eventually be a standard feature.

Depending on the strawman that gets standardized it will either be .then(doneFn, failFn); or .when(doneFn, failFn);

Though these promises aren't going to have something like .progress().
Comment 3 Krinkle 2012-08-03 09:17:45 UTC
(In reply to comment #2)
> Note that es-harmony seems to be standardizing promises in such a way they'll
> eventually be a standard feature.
> 
> Depending on the strawman that gets standardized it will either be
> .then(doneFn, failFn); or .when(doneFn, failFn);
> 
> Though these promises aren't going to have something like .progress().

Interesting, but lets not fall into the pitfall of bad coding patterns under the excuse of familiar patterns from unrelated environments.

We are using a jQuery promise, which should be used the way a jQuery promise should be used. Even if that means it won't look "like" an es-harmony promise. 

I'm not saying .then() will fail on a jQuery promise, but that's not how I think we should encourage usage of it.

At least what es-harmony will or will not do can not possibly play a role in this decision.
Comment 4 denny vrandecic 2012-08-08 20:22:23 UTC
changed accordingly: https://gerrit.wikimedia.org/r/#/c/17794/ -- by an external volunteer developer! Yay Joancreus!
Comment 5 Anja Jentzsch 2012-11-29 12:38:33 UTC
Verified in Wikidata demo time for sprint 11

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links