Patch 142 #145
Patch 142 #145
Conversation
Although it is not a good long-term solution.
R/drop_file_ops.R
Outdated
| @@ -193,7 +193,7 @@ drop_delete <- | |||
| if (verbose) { | |||
| res | |||
| } else { | |||
| message(sprintf('%s was successfully deleted', res$metadata$path_display)) | |||
| # message(sprintf('%s was successfully deleted', res$metadata$path_display)) | |||
There was a problem hiding this comment.
why keep the old message around?
| @@ -21,8 +21,9 @@ test_that("Test that basic file ops work correctly", { | |||
| server_row_count <- nrow(y) | |||
| # Make sure the downloaded csv has the same number of rows | |||
| expect_equal(row_count, server_row_count) | |||
| expect_message(drop_delete(file_name), "successfully deleted") | |||
| # expect_message(drop_delete(file_name), "successfully deleted") | |||
There was a problem hiding this comment.
should probably just drop this
DESCRIPTION
Outdated
| @@ -1,6 +1,6 @@ | |||
| Package: rdrop2 | |||
| Title: Programmatic Interface to the 'Dropbox' API | |||
| Version: 0.8.1.9999 | |||
| Version: 0.8.2.9999 | |||
There was a problem hiding this comment.
we don't need to bump the version here, do we? this implies 0.8.2 already exists, but it doesn't. I think the point of .9999 is to say "caution, this stuff is in flux" and avoid some complicated gitflow-like workflow with release branches and all that
There was a problem hiding this comment.
Because the PR is a patch? My rationale: If someone files a similar issue but have not updated to this change, we cannot tell if they have this fixed version or the last cran release. We could see the hash if they give us devtools::session_info but not otherwise.
What would you suggest as a workflow for bumping minor? Just with each release?
There was a problem hiding this comment.
That's a good point. Looking at other packages (dplyr, purrr, httr) looks like they only go from "real" 3-number releases to 3-number-plus-9000 (not 9999) and back again, keeping the .9000 version for some time.
I get wanting to tell where users are when they file issues, but bumping the version everytime we push to master seems like too much; we might quickly be at 8.2.99.9999, and it invites a plethora of merge conflicts since every PR will change that line.
I definitely lean towards leaving the version until we're ready for a CRAN release or switching back to development, but we don't have to do that.
|
@ClaytonJY Fair enough re: the version plan. |
|
@ClaytonJY All comments addressed, this is ready to merge. |
|
@karthik the code addresses #142, but the test doesn't, though it's still a good test to add. The issue was about getting an unhelpful HTTP error when trying to make something in a folder that doesn't exist yet; now that they should get |
This should fix #142
Fixed
drop_exists.