Made function factories to recreate the auth and deauth functions in …

[jonthegeek]

…client packages. Needs to be tested in context of other packages before I'm 100% confident it works, but it seems to work in principle. The environment of the resulting auth function is the only thing I'm not 100% certain I nailed. Closes #57.

@jennybc This is what I mean. What do you think? It takes some of the copy/paste instability out of making auth/deauth functions in client packages.

[jennybc]

I can't review in depth now but I will be working on gargle again soon. But fwiw:

The environment of the resulting auth function

and simplicity / maintainability is why my instinct was to not go down this road.

[jonthegeek]

I can understand that instinct... but I'm pretty certain this will work as I expect, and, if so, it makes gargle way easier to implement. After my R4DS office hour I'll test it out in a package and make sure it works as expected.

[jennybc]

Note that the user of the wrapper package needs to be able to specify scopes.

[jonthegeek]

The user can specify! The returned function has a scopes argument, which defaults to what the package creator sets, but can be changed by the user.

It looks like R 3.1 doesn't like something I used in this, so that part is unfortunate. I'll see if I can track it down. Edit: That's not related to this change.

[jonthegeek]

I forked googlesheets4 and replaced the sheets_auth definition with this:
sheets_auth <- gargle::make_auth( pkg_name = "googlesheets4", scopes = "https://www.googleapis.com/auth/spreadsheets", error_message = paste( "Can't get Google credentials.\n", "Are you running googlesheets4 in a non-interactive session? Consider:\n", " * sheets_deauth() to prevent the attempt to get credentials.\n", " * Call sheets_auth() directly with all necessary specifics.\n" ) )

Before and after I did that, 1 test failed; the rest all passed. As far as I can tell, it generates exactly the same function.

I understand if you end up rejecting this PR, but I really feel like it makes it easier for users to implement gargle.

[jennybc]

OK I'll just have to think about it.

This is something else to consider:

We try to avoid creating install or build time dependencies between package A and the version of package B. This has caused problems in the past, when package B updates but package A does not. So you really want package A to be calling into package B, not calling a function defined using the version of package B installed when package A was installed. This is, for example, why devtools exposes functions in, e.g., usethis and pkgload the way it does. I didn't explain that very well, but maybe you can figure out what I'm saying.

[jonthegeek]

I could be wrong, but this feels like a special case. And to be clear, it isn't loading the function from gargle; the function from gargle returns a function tailored to the package via the arguments to the gargle function. I put the unique googledrive and googlesheets4 auth functions in the tests as examples to hopefully make it clear.

[jonthegeek]

An alternative that might feel less scary/wrong: What about a usethis-like function that writes auth.R (or {yourpackage}_auth.R) in your package's R folder, using arguments to the function to fill in all the things that vary package-to-package as much as possible? That file is so similar package to package, it'd be nice to have something to help set it up. The arguments would be ~the things in the gargle_lookup_table. Possibly then it could/would write all the help documentation without arguments to gargle, making it easier to customize from there? And maybe include a comment in there about how to rebuild it (ie, the original call)?

[jennybc]

Yes, a usethis-like templating function is already on my radar. But first I'm gaining more experience by wiring gargle into several packages to see how things work.

[jennybc]

Thanks for exploring this idea. It is appealing conceptually, for sure.

But I'm sticking with the current approach that's based more on templating and documentation, with full acknowledgement that it's not terribly elegant.

I am somewhat concerned with too much abstraction and maintainability.

But the main reason we didn't walk down this road is that this approach needs even more indirection to make it safe. We don't want to create build-time dependencies between gargle and client packages that may use it. Here are some links about this pretty subtle, insidious gotcha:

https://r6.r-lib.org/articles/Portable.html#potential-pitfalls-with-cross-package-inheritance

hadley/r-pkgs#550

r-lib/devtools#1788

[jonthegeek]

No problem! This exploration led me down a road that has me building a package for more efficient function-factory creation (https://github.com/jonthegeek/factory), so I'll be sure to take those caveats to heart!