Skip to content
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

Merged
merged 12 commits into from Jul 27, 2017
Merged

New PR for #460

merged 12 commits into from Jul 27, 2017

Conversation

muschellij2
Copy link
Contributor

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:

#' GET("https://jeroenooms.github.io/data/diamonds.json",
as that link was dead and causing errors in the "examples" part of R CMD check

@hadley
Copy link
Member

hadley commented Jul 26, 2017

Can you please move the write_stream stuff to a separate PR?

@muschellij2
Copy link
Contributor Author

OK - this is split. After merged - I will fetch, then send new PR.

@muschellij2
Copy link
Contributor Author

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}}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Member

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


# 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,
Copy link
Member

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

@muschellij2
Copy link
Contributor Author

Quite the hassle for such a small PR! It's stylized now.

Copy link
Member

@hadley hadley left a 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"),
Copy link
Member

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
)

Copy link
Contributor Author

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?

@hadley hadley merged commit 20b26b1 into r-lib:master Jul 27, 2017
@hadley
Copy link
Member

hadley commented Jul 27, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants