philschatz / octokat.js

:octocat: Github API Client using Promises or callbacks. Intended for the browser or NodeJS.
http://philschatz.com/2014/05/25/octokat/
MIT License
421 stars 133 forks source link

document how to specify parameters, such as when filtering repo issues #75

Open sam-github opened 8 years ago

sam-github commented 8 years ago

I found this clue here: https://github.com/philschatz/octokat.js/issues/69#issuecomment-142646133, which got me going, but until then all my attempts were failures:

octo.repos('strongloop-internal', 'scrum-nodeops')                                             
//.issues({labels: inSprintLabels})                                                            
//.issues.parameters({labels: inSprintLabels})                                                 
//.issues.labels(inSprintLabels)                                                               
.issues.fetch({labels: inSprintLabels}, function(err, issues) { // <--- yeah!
philschatz commented 8 years ago

Thank you for noting the attempts! I'll try to add more documentation and then either add errors for some of the options or actually, maybe just support at least the 1st option without breaking the existing API.

For example:

.issues({labels: inSprintLabels}).fetch(cb)

// Useful if you need to provide an id as well as params
.issues(12, {labels: inSprintLabels}).fetch(cb) 

// and maybe this one. But that would rely on GitHub not introducing `params` in the URL
.issues.params({labels: inSprintLabels}).fetch(cb)
sam-github commented 8 years ago

I actually think the API is fine, it even makes sense that parameters would go somewhere other than the path, and I for one don't need multiple cosmetically different but identical in function APIs, so don't do it on my behalf! :-)

There were error messages for everything I tried, they didn't fail silently or anything terrible like that.

Btw, I quite appreciate how closely the API follows the github API docs, the last package I used had too many gratuitous differences. If you go the way of API sugar, its suddenly necessary to exhaustively document all the sugar, I think your way of describing the mapping is basically better, it just needs a couple more paragraphs.

Also, I believe you are renaming returned object properties? For example, closed_at renamed to closedAt? I didn't see that documented, and it surprised me when octokat's output didn't match the github API docs in this regard.