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.

Sign up for GitHub

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

Jump to bottom

Adds a new --acl option that allows to set the canned ACL to be applied to the object #121

Closed
wants to merge 1 commit into from
Closed

Adds a new --acl option that allows to set the canned ACL to be applied to the object #121

wants to merge 1 commit into from

Conversation

monken
Copy link
Contributor

@monken monken commented Oct 13, 2016

Default is off. Allowed values are private | public-read |
public-read-write | authenticated-read | aws-exec-read | bucket-owner-read
| bucket-owner-full-control.

@monken
Copy link
Contributor Author

monken commented Oct 14, 2016

@kahing I just rebased it off of master so it should merge cleanly.

@kahing
Copy link
Owner

kahing commented Oct 17, 2016

should this be --canned-acl or --acl-canned instead of just --acl? also, what do you think about prefixing aws specific options with --aws-blah? There are many S3 implementations that don't support all these aws features and grouping them may make --help easier to read (or at least group them together in a different section)

Also, could you change the possible values display to follow the convention in the other flags? (Possible values: a, b, c instead of allowed values: a | b | c)

@kahing kahing mentioned this pull request Oct 17, 2016
Merged
@monken
Copy link
Contributor Author

monken commented Oct 17, 2016

I agree that the aws prefix could make it cleaner. However, the sse and sse-kms flags are not supported by services other than S3 either. I tried to be consistent with the implementation of that feature.

It's your call, I'm happy to change it :)

@monken
Copy link
Contributor Author

monken commented Oct 17, 2016

Also, could you change the possible values display to follow the convention in the other flags? (Possible values: a, b, c instead of allowed values: a | b | c)

Done

@monken
Adds a new --acl option that allows to set the canned ACL to be appli…
e864fea 
…ed to

the object. Default is off. Allowed values are private | public-read |
public-read-write | authenticated-read | aws-exec-read | bucket-owner-read
| bucket-owner-full-control.
@kahing
Copy link
Owner

kahing commented Oct 19, 2016

merged as ac4f3b6 . amended help text

@kahing kahing closed this Oct 19, 2016
@monken monken mentioned this pull request Nov 2, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@monken @kahing