Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made callbacks optionnal #296

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Made callbacks optionnal #296

wants to merge 1 commit into from

Conversation

vinz243
Copy link

@vinz243 vinz243 commented Oct 12, 2013

Made callbacks optional thus it will avoid some errors or bugs

Made callbacks optional thus it will avoid some errors or bugs
@OscarGodson
Copy link
Owner

Thanks! A few things:

  1. Our style is to do callback = callback || function () {} at the beginning of the functions. Could you do that instead, or, at the least add curlys around those. We always add curlys in our codebase.
  2. Do all tests pass? While this is not required, it'd rock if you added simple test cases for this (if callback is omitted, an error is NOT thrown)
  3. Whenever you're done with 1 and 2 please build (jake build to build manually or jake watch to build while editing)

@zethussuen
Copy link
Collaborator

Seems @vinz243 has removed his original PR branch. I can take a look into refactoring the code and adding tests, unless he's still interested?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants