Permalink
Browse files

travis-841337: Don't include data in Output.resolve

Change 79a1a96 (just 5 commits above) added a promise to the Output
object, which resolves when the execution finishes with the output of
the command. This sounds like a good idea, but in practice it's
confusing:
- The requisition exec resolves to an output object, but when you're
  chaining promises it's easy to forget what you're waiting on
- The data often isn't useful by itself - you're likely to need to know
  the type of the data, or if it's an error, etc. So you're likely to
  need the output object anyway.

You could make an argument for resolve(this), but I'd want to have a
play first, and using undefined is safe - we can expose something else
instead, but we can't take away.

Signed-off-by: Joe Walker <jwalker@mozilla.com>
  • Loading branch information...
1 parent d4aff12 commit 87337a9960d11827b46deca377323a272a53ca46 @joewalker joewalker committed Feb 18, 2013
Showing with 2 additions and 2 deletions.
  1. +2 −2 lib/gcli/cli.js
View
@@ -1779,10 +1779,10 @@ Output.prototype.complete = function(data, error, ev) {
this.changed(data, ev);
if (error) {
- this.deferred.reject(data);
+ this.deferred.reject();
}
else {
- this.deferred.resolve(data);
+ this.deferred.resolve();
}
};

0 comments on commit 87337a9

Please sign in to comment.