-
Notifications
You must be signed in to change notification settings - Fork 219
Adding support for shared_link password parameter #66
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
Conversation
|
Hi @Frencil, thanks for the pull request. Before we can merge it, we need you to sign our Contributor License Agreement. You can do so electronically here: http://opensource.box.com/cla Once you have signed, just add a comment to this pull request saying, "CLA signed". Thanks! |
|
CLA signed |
|
Verified that @Frencil has just signed the CLA. Thanks, and we look forward to your contribution. |
boxsdk/object/item.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? I thought that not specifying a password creates the shared link with no password.
Also, can you mention here that setting passwords is a premium feature, as is https://github.com/box/box-python-sdk/blob/master/boxsdk/object/folder.py#L192
|
LGTM with my 2 comments addressed. |
- moved password argument to end of argument list - updated param doc to be correct in the event of no password - updated param doc to notify that this is a premium feature
|
Thanks for the feedback! FYI I see the Travis build failing just on the pypy environment because of this:
FWIW this was one error I also had locally (before modifying anything) and couldn't figure out a solution for. Since none of the changes in this branch affected that part of the test coverage I decided to proceed anyway, but if anyone knows how to fix that I'd love to get it fixed locally too. |
|
The PyPy failures look like a regression in the Downgrading the package to version 0.9.3 with |
boxsdk/object/item.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last thing - these 2 lines need to move down in the docstring as well!
|
Not sure how I missed that last one, thanks. =) |
Adding support for shared_link password parameter
Resolves Issue #65 by adding support for shared_link passwords.