Bug 846959 - Ensure syncComplete event fires and allow callback in syncC... #8415
Conversation
| @@ -44,9 +44,14 @@ Calendar.ns('Controllers').Sync = (function() { | |||
| * controller.once('syncComplete', cb); | |||
| * | |||
| */ | |||
| all: function() { | |||
| all: function(callback) { | |||
| if (callback) { | |||
There was a problem hiding this comment.
Wouldn't it be cleaner to always just rely on the syncComplete function instead of callback? I suppose this is a smaller fix for now, but it seems odd to have two hooks IMO.
There was a problem hiding this comment.
Needed to fix across all branches... stupid of me really... less mocking in tests required.
There was a problem hiding this comment.
Cool - let's go with this for now then.
There was a problem hiding this comment.
less mocking in tests required.
or better mocking ? :)
There was a problem hiding this comment.
Both!!! One of the things I liked about Sinon is it would alert you of stupid mistakes like missing function or argument mis matches...
On Mar 2, 2013, at 1:09 PM, Julien Wajsberg notifications@github.com wrote:
In apps/calendar/js/controllers/sync.js:
@@ -44,9 +44,14 @@ Calendar.ns('Controllers').Sync = (function() {
* controller.once('syncComplete', cb);
*
*/
- all: function() {
- all: function(callback) {
if (callback) {
or better mocking ? :)—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
Yep, I'd like to try Sinon as well :)
Bug 846959 - Ensure syncComplete event fires and allow callback in syncController r=kgrandon
Bug 846959 - Ensure syncComplete event fires and allow callback in syncController r=kgrandon(cherry picked from commit 22ab6a7)
Bug 846959 - Ensure syncComplete event fires and allow callback in syncController r=kgrandon(cherry picked from commit 22ab6a7)
...ontroller.all