-
Notifications
You must be signed in to change notification settings - Fork 16
Fix functions' callback with hardcoded 'null' as error parameter #56
Conversation
@@ -82,7 +82,7 @@ class Model | |||
# Returns null | |||
@save: (id, data, callback) -> | |||
@adapter.save id, data, (err, attributes) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a return callback err is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameters are given to the callback as-is, what is missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that you return something when an error occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes sorry, I didn't realize it was up to your quote! It's fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry but you didn't change anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did, but since I reverted a few lines it's not showing up anymore in the diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the problem is not solved. You should add a
return callback err if err
before this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's why I didn't understand…it seems a bit overzealous to me to do in two lines what is straightforward in one, especially since the purpose of this way of handling errors is to prevent conditional nesting, which is absent here. I'm doing the change though. Edit: fixed.
Thank you @jsilvestre |
Fix functions' callback with hardcoded 'null' as error parameter
The small incoherence bothered me while documenting, so I'm suggesting a fix.