-
Notifications
You must be signed in to change notification settings - Fork 83
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
Response status codes #22
Conversation
CLA is valid! |
969ed69
to
5976b5e
Compare
* @param {Object} params The parameters identify the resource, and along with information | ||
* carried in query and matrix parameters in typical REST API | ||
* @param {Object} [context={}] The context object. It can contain "config" for per-request config data. | ||
* @param {Function} callback callback convention is the same as Node.js |
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.
Could you change the docblocks since we're no longer using the same convention as node with the addition of the 3rd meta param?
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.
Sure thing.
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.
What are you using to generate docs so I can look up the syntax for defining callbacks? I ask because it looks like jsdoc/jsdoc#260 addresses a syntax for this, but I don't know which approach to use.
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.
Something like this: http://usejsdoc.org/tags-callback.html ?
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.
Do you want me to update the README as well since it shows only the two parameters on the callback?
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.
Yeah, we use jsdoc. Since @callback
is the official way to go, that sounds fine to me.
We probably don't use the @callback
as much as it needs to be through out the rest of our code :)
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.
Go for it, update the README too
🚢 This looks good. After merge, I will create better docs for the data-fetchers with the callback format specified. |
I have updated the jsdocs comments. Let me know if the wording needs any tweaks. |
We actually use YUI Doc for our API documentation, which uses a lot of the same syntax as jsdoc. We can clean those up in a separate commit after we merge this. |
Sounds good, I'll leave that bit to y'all then. Is there anything else that I need to cleanup? |
Looks good @mtscout6, thanks! |
Awesome! Thanks @mtscout6! |
No problem |
Related to #17