-
Notifications
You must be signed in to change notification settings - Fork 309
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
Adding pagination docs for clients/connections #268
Conversation
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.
Also ... should there be tests added for these parameters?
src/management/ClientsManager.js
Outdated
* </caption> | ||
* | ||
* // Pagination settings. | ||
* var params = { |
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 would include what the defaults are, i.e. what happens if these are not passed (page 1, 50 per page I believe)
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 won't add that because it's a backend default and might change
src/management/ClientsManager.js
Outdated
* management.clients.getAll(function (err, clients) { | ||
* console.log(clients.length); | ||
* }); | ||
* | ||
* @param {Function} [cb] Callback function. | ||
* @param {Object} [params] Users params. |
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.
Users?
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.
fixed
src/management/ConnectionsManager.js
Outdated
* management.connections.getAll(function (err, connections) { | ||
* console.log(connections.length); | ||
* }); | ||
* | ||
* @param {Function} [cb] Callback function. | ||
* @param {Object} [params] Users params. |
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.
Users?
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.
fixed
src/management/ConnectionsManager.js
Outdated
* </caption> | ||
* | ||
* // Pagination settings. | ||
* var params = { |
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.
Same comment here, show what the defaults are if this is not provided
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.
same
src/management/ClientsManager.js
Outdated
* @example | ||
* @example <caption> | ||
* This method takes an optional object as first argument that may be used to | ||
* specify pagination settings and the search query. |
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 search query
?
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.
fixed
src/management/ConnectionsManager.js
Outdated
* @example | ||
* @example <caption> | ||
* This method takes an optional object as first argument that may be used to | ||
* specify pagination settings and the search query. |
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 search query
?
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.
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.
⛴
No description provided.