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

Introduce the general idea of overwrite NA/TRUE/FALSE #266

Merged
merged 25 commits into from
Aug 14, 2019

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Aug 10, 2019

Closes #230

I decided that, if overwrite happens anywhere (such as in drive_upload(), it should happen for the entire set of functions that create new files.

drive_put() is also new.

In previous refactoring commit, I stopped checking whether both `path` and `parents` were specified in `drive_upload()`. I claim this is incredibly rare and not worth the code and headspace to maintain.

Let the behaviour be undefined / undescribed and, if it is wrong or surprising and actually matters, someone will speak up.
Tests added in the next commit revealed that, for filepath input to `drive_rm()`, we tried to delete each file n times, where n = the number of filepaths for that file. n can be greater than 1 if a file has multiple parents.
I am removing checks and tests around unusual parent situations elsewhere. I intend to send people to drive_update() for all weird parent folder situations. So I want an explicit test that you can add a parent folder this way.
Revisit this later with lifecycle
@jennybc jennybc changed the title Introduce the general idea of overwrite TRUE/FALSE Introduce the general idea of overwrite NA/TRUE/FALSE Aug 10, 2019
@jennybc
Copy link
Member Author

jennybc commented Aug 10, 2019

@LucyMcGowan This is the biggest piece of work in googledrive since its release (other than the adoption of gargle, which didn't really add or change a Drive feature per se).

I still need to add tests for this and make sure overwrite appears in some examples.

And I can fly solo here. But if you are in a position to look this over, it is always great to have another set of eyeballs. Even if you just read and comment, that's welcome.

@jennybc jennybc mentioned this pull request Aug 13, 2019
@LucyMcGowan
Copy link
Member

I took a quick look, and this looks awesome! The only thing I noticed is we already had an overwrite parameter for drive_download() that is a bit different since it can just be TRUE/FALSE (since the local file systems aren't as wonky as the Drive ones 😂) - do you think we should update that wording to match-ish (with the exception of NA) or at least point out the distinction?

@jennybc
Copy link
Member Author

jennybc commented Aug 14, 2019

Thanks @LucyMcGowan, that's a good point. I added a "local" qualifier there, to serve as a reminder of where the overwrite could occur.

@jennybc jennybc merged commit 60f337b into master Aug 14, 2019
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.

Overwrite control
2 participants