-
Notifications
You must be signed in to change notification settings - Fork 2k
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
New PR for #460
New PR for #460
Conversation
Merge remote-tracking branch 'upstream/master'
Can you please move the |
OK - this is split. After merged - I will fetch, then send new PR. |
All checks passed - ready to merge. |
R/oauth-init.R
Outdated
@@ -62,12 +62,14 @@ init_oauth1.0 <- function(endpoint, app, permission = NULL, | |||
#' retrieve the token. Some authorization servers require this. | |||
#' If \code{FALSE}, the default, retrieve the token by including the | |||
#' app key and secret in the request body. | |||
#' @param config_init Additional configuration settings sent to | |||
#' \code{\link{POST}}, e.g. \code{\link{user_agent}}. |
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.
Needs to be indented - http://style.tidyverse.org/code-documentation.html#indention
R/oauth-init.R
Outdated
@@ -109,10 +111,11 @@ init_oauth2.0 <- function(endpoint, app, scope = NULL, user_params = NULL, | |||
|
|||
if (isTRUE(use_basic_auth)) { | |||
req <- POST(endpoint$access, encode = "form", body = req_params, | |||
authenticate(app$key, app$secret, type = "basic")) | |||
authenticate(app$key, app$secret, type = "basic"), config = config_init) |
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.
Can you please convert all the calls you touched to use tidyverse style - http://style.tidyverse.org/syntax.html#long-lines
demo/oauth2-reddit.R
Outdated
|
||
# 3b. If get 429 too many requests, the default user_agent is overloaded. | ||
# If you have an application on Reddit then you can pass that using: | ||
token <- oauth2.0_token(reddit, app, |
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 only use two spaces to indent - http://style.tidyverse.org/syntax.html#long-lines
Quite the hassle for such a small PR! It's stylized now. |
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 know this feels like a lot of work, but I have to maintain the code for ever after.
R/oauth-init.R
Outdated
@@ -109,10 +111,12 @@ init_oauth2.0 <- function(endpoint, app, scope = NULL, user_params = NULL, | |||
|
|||
if (isTRUE(use_basic_auth)) { | |||
req <- POST(endpoint$access, encode = "form", body = req_params, | |||
authenticate(app$key, app$secret, type = "basic")) | |||
authenticate(app$key, app$secret, type = "basic"), |
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 still isn't indented correctly. It should look like
req <- POST(endpoint$access,
encode = "form",
body = req_params,
authenticate(app$key, app$secret, type = "basic"),
config = config_init
)
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've made the edits and pushed. Why is RStudio Reindent-Lines
not tidyverse
-style-compatible?
Thanks! |
This replaced #411 PR. Removed extraneous .rproj edits and such.
Once checks are done - this can be merged.
Fix reddit demo where user_agent was being overloaded. Now can pass user_agent using config_init to init_oauth2.0 and Token2.0$new() (@muschellij2 @hadley #363 and #413).
Changed example in write_stream from https://jeroenooms.github.io/data/diamonds.json to https://github.com/jeroen/data/raw/gh-pages/diamonds.json as this had moved (@muschellij2).
Also, fixed an example on write_stream:
httr/R/write-function.R
Line 61 in 1078b1d