-
Notifications
You must be signed in to change notification settings - Fork 719
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 ElasticSearchService Domain and SQS Queue #84
Conversation
Updated SqsQueue to SQSQueue for consistency |
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.
Thank you for adding all the resources! Greatly appreciated.
resources/sqs-queues.go
Outdated
|
||
type SQSQueue struct { | ||
svc *sqs.SQS | ||
queueurl *string |
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.
Sorry for being the pedantic one, but would you use queueURL
to follow golang style conventions? The AWS SDK does not follow it, but we decided to follow it since it is the de-facto standard.
resources/sqs-queues.go
Outdated
register("SQSQueue", ListSqsQueues) | ||
} | ||
|
||
func ListSqsQueues(sess *session.Session) ([]Resource, error) { |
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.
Same as above: ListSQSQueues
|
||
type ESDomain struct { | ||
svc *elasticsearchservice.ElasticsearchService | ||
domainname *string |
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.
Consider: domainName
Updated these, Ill check my next PR now |
@tomvachon Sorry for the late response. Your PRs look good so far. Could you squash them, so there is only one commit per PR and no merge commit? That would make the testing easier for us. |
Yea, I can. Should I do it retroactively or going forward?
…On Mon, Mar 5, 2018 at 8:51 AM, Sven Walter ***@***.***> wrote:
@tomvachon <https://github.com/tomvachon> Sorry for the late response.
Your PRs look good so far. Could you squash them, so there is only one
commit per PR and no merge commit? That would make the testing easier for
us.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#84 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAUQRaqu--V8kV6lRBddDFA4XYkAzLBlks5tbULkgaJpZM4SZ9Uj>
.
|
Doing it retroactively would be nice. |
Give me a few minutes, I'm trying to remember how to do this again ;) |
@svenwltr my head isn't functioning, how do I squash them again, sorry for the dumb question |
|
Ok, squashing now, I had to rebase master to avoid the merge commit too |
Thank you very much. We will review and merge soon. |
Note, deletion of ESS Domains is very long in wait times