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 file/folder sharing feature #37

Merged
merged 2 commits into from Mar 11, 2019
Merged

Conversation

raghav-dalmia
Copy link
Contributor

@raghav-dalmia raghav-dalmia commented Mar 9, 2019

I write a function a function name share() in action.py, which take file id as mandatory argument and role, type and message are it's optional arguments.

Following image show result of drive share --help

drive share --help

If type is anyone, it will generate a share link and print it, otherwise, it will take email id as an argument and share the link directly to grantee/user.

@inishchith @nurdtechie98, please review.

Fixes #15

@nurdtechie98
Copy link
Owner

@raghav-dalmia please remove the merge conflict.

@raghav-dalmia
Copy link
Contributor Author

Done @nurdtechie98

drive_cli/actions.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@inishchith inishchith left a comment

Choose a reason for hiding this comment

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

  • I've tested this locally.

@raghav-dalmia Great work. Overall looks good to me, just some minor changes. As soon as you make the changes. I'll merge this one. Thanks for your time :)

drive_cli/actions.py Outdated Show resolved Hide resolved
drive_cli/actions.py Outdated Show resolved Hide resolved
drive_cli/actions.py Outdated Show resolved Hide resolved
drive_cli/actions.py Outdated Show resolved Hide resolved
@raghav-dalmia
Copy link
Contributor Author

Done @inishchith

Copy link
Owner

@nurdtechie98 nurdtechie98 left a comment

Choose a reason for hiding this comment

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

@raghav-dalmia other than changes said by @inishchith, make sure you code is pep8 compliant, cos I see a lot of errors on running flake8 test. Rest looks good to me.

@raghav-dalmia
Copy link
Contributor Author

I fix my code using autopep8, please check. Sorry about this thing.

@inishchith
Copy link
Collaborator

@raghav-dalmia can you please sync your branch. As i see you've changes which are irrelevant to the task.

@raghav-dalmia
Copy link
Contributor Author

Sorry I forgot to update my local repo, that's why this happened. Now, I think it is perfect. Please review.

@nurdtechie98
Copy link
Owner

@raghav-dalmia still has some warnings in flake8 test:

./drive_cli/actions.py:298:25: W291 trailing whitespace
./drive_cli/actions.py:299:35: W291 trailing whitespace
./drive_cli/actions.py:305:33: W291 trailing whitespace
./drive_cli/actions.py:308:53: W291 trailing whitespace
./drive_cli/actions.py:319:79: W291 trailing whitespace

@raghav-dalmia
Copy link
Contributor Author

Now it's fine. Please review.

Copy link
Collaborator

@inishchith inishchith left a comment

Choose a reason for hiding this comment

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

LGTM ;)

Copy link
Owner

@nurdtechie98 nurdtechie98 left a comment

Choose a reason for hiding this comment

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

@raghav-dalmia good job, LGTM merging.

@nurdtechie98
Copy link
Owner

closes #15

@nurdtechie98 nurdtechie98 reopened this Mar 11, 2019
@nurdtechie98 nurdtechie98 merged commit 783ae62 into nurdtechie98:dev Mar 11, 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.

None yet

3 participants