-
Notifications
You must be signed in to change notification settings - Fork 560
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 commands for managing Bucket CORS #5019
Add commands for managing Bucket CORS #5019
Conversation
df83db8
to
1fae1d1
Compare
1fae1d1
to
fb27613
Compare
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.
Looks solid.
} | ||
|
||
// corsMessage container for output message. | ||
type corsMessage struct { |
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.
Since this msg is used by all commands.. maybe it and it's methods should live in the cord-main.go file.
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.
@zveinn Good point, and I agree, but I put corsMessage in here to be consistent with the other commands that share a message struct, here are three examples:
- https://github.com/minio/mc/blob/master/cmd/admin-user-add.go#L93-L102
- https://github.com/minio/mc/blob/master/cmd/admin-group-add.go#L66-L74
- https://github.com/minio/mc/blob/master/cmd/ilm-tier-add.go#L346-L356
I am totally happy to move it, just give me the 👍 and ack that it makes it inconsistent with the rest of the commands.
Could also make a follow up PR to move them all. I don't know why it's like this.
d03d0bc
to
8c10b67
Compare
8c10b67
to
1e53784
Compare
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.
LGTM
Community Contribution License
All community contributions in this pull request are licensed to the project maintainers
under the terms of the Apache 2 license.
By creating this pull request I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 license.
Description
Add the following commands:
mc cors set
mc cors remove
mc cors get
Motivation and Context
These will work with S3 and MinEOS
How to test this PR?
Types of changes
Checklist:
commit-id
orPR #
here)