rdrop2 icon indicating copy to clipboard operation
rdrop2 copied to clipboard

Separate API calls to internal functions.

Open ClaytonJY opened this issue 7 years ago • 2 comments

All functions except drop_auth, drop_download, and drop_upload now use an internal API wrapper (named like api_*) to interact with the Dropbox API.

I put all the functions in the relevant file, at the end. Standardized usage with a new post_api utility function.

Fixes #109.

ClaytonJY avatar Oct 18 '17 05:10 ClaytonJY

Nice! I was going to start on this over the weekend/early next week but I'll review this Monday or Tuesday. 🎈

karthik avatar Oct 19 '17 00:10 karthik

@karthik cool, no rush. Some additional notes:

  • no end-user apparent changes here, just refactoring.
  • added tibble as an import so I could use lst; doesn't actually add a dependency since we import dplyr
  • I picked api_* for the common name format of the API functions; pretty arbitrary, but they're internal, I liked keeping them separate from the drop_*'s
  • post_api is kinda weird; more than open to new names. Arg order is different, but I didn't want to put the token after the ... so I didn't have to sweat naming that arg every time
  • in the long-run, we should probably have strong assertions in the api_ functions; skipped for now both to minimize effort and keep this PR manageable. Save that for the new package.
  • probably kills test coverage; I think most of this code was already hit in tests, but now it takes up fewer lines. PR is only net-positive on lines because of barebones function comments on new funcs.
  • didn't put any real thought into doing anything with drop_auth/drop_upload/drop_download here; much trickier. Could probably share some code between upload/download, not sure about auth. Maybe post_api should have another sub-function that's even less opinionated, and could be used to build auth/upload/download requests too?

ClaytonJY avatar Oct 19 '17 19:10 ClaytonJY