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

add copy method to drive.py #188

Merged
merged 19 commits into from
Jul 19, 2022
Merged

Conversation

simone-viozzi
Copy link
Contributor

Hi i added the copy method keeping it as simple as possible.

this PR fixes #83 #53

pydrive2/drive.py Outdated Show resolved Hide resolved
pydrive2/drive.py Outdated Show resolved Hide resolved
Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've left a comment. Primarily next step is to move it to the GoogleDriveFile class and we'll need to add tests.

pydrive2/files.py Outdated Show resolved Hide resolved
@simone-viozzi simone-viozzi requested a review from shcheklein July 5, 2022 11:13
pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

good progress :) please, see some comments + we still need to add tests for this

@simone-viozzi
Copy link
Contributor Author

simone-viozzi commented Jul 6, 2022

Where the test should go? on test_file.py or test_fs.py?
they have very different structure

also, I can't seem to run the existing ones :) but is most likely because of the authentication.

@simone-viozzi simone-viozzi requested a review from shcheklein July 7, 2022 18:21
pydrive2/drive.py Outdated Show resolved Hide resolved
@simone-viozzi simone-viozzi requested a review from shcheklein July 12, 2022 15:28
pydrive2/drive.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

Good progress! Please take a look at the latest review.

@simone-viozzi simone-viozzi requested a review from shcheklein July 13, 2022 21:17
@shcheklein
Copy link
Member

The documentation of the function itself are generated from the doc-string, right?

Yes, but I would see if there are mentions in those rsts. If not - that's fine.

If you can do a quick example - that would be great! If not that's also fine- not a blocker. Just let me know when you feel you are ready with that, and we'll merge it.

@shcheklein shcheklein temporarily deployed to external July 19, 2022 01:26 Inactive
@shcheklein shcheklein temporarily deployed to external July 19, 2022 04:10 Inactive
@simone-viozzi
Copy link
Contributor Author

@shcheklein let's merge it like this. I plan to add other methods too like rename and move, so maybe I will do an example use case with those too.

@shcheklein shcheklein merged commit 4a6590c into iterative:master Jul 19, 2022
@shcheklein
Copy link
Member

@simone-viozzi done, thanks for the PR.

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.

Add copy method wrapper to the file API
2 participants