-
Notifications
You must be signed in to change notification settings - Fork 51
adds ability to pass request default options #91
Conversation
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "flickrapi", | |||
"version": "0.5.0", | |||
"version": "0.5.1", |
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.
please leave this as is - it is managed through the npm version
command. (Also note that as new functionality, this would be a minor version update, definitely not a bugfix/patch update)
I don't see an issue that describes a need that this PR addresses, nor any documentation changes in this PR that describes the new functionality, so could I ask you to please file an issue that this PR is a solution for, and amend the PR itself with changes to the README.md to explain the new functionality? |
ok. will do that |
@@ -48,7 +48,7 @@ module.exports = (function Flickr() { | |||
signature = Utils.sign(data, options.secret, options.access_token_secret), | |||
flickrURL = url + "?" + queryString + "&oauth_signature=" + signature; | |||
|
|||
request.get(flickrURL, function(error, response, body) { | |||
request.defaults(options.requestOptions).get(flickrURL, function(error, response, body) { |
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.
this code implies that options.requestOptions
are way wider than just being for setting the request timeout. We should either have users supply the timeout as dedicated option and then pass that in here, using
request.defaults({ timeout: options.requestTimeout }).get(....)
or we should document that the options.requestOptions
can be used to set any and all values that are accepted by the request.defaults()
function
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 the later will be better.
@@ -94,6 +94,27 @@ Flickr.authenticate(flickrOptions, function(error, flickr) { | |||
}); | |||
``` | |||
|
|||
### As flickrapi internally uses the request module, you can also pass default options for request that are accepted by request.defaults() as documented in the [request module](https://github.com/request/request#convenience-methods) |
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.
let's point explicitly to https://github.com/request/request#requestdefaultsoptions here, since we push those options into defaults()
, specifically.
I have updated the PR |
great, I will try to get to this before the end of the day! |
Hi @Pomax, Can you please take a relook at the PR. Thanks! |
merge'd |
No description provided.