-
Notifications
You must be signed in to change notification settings - Fork 26
Multiaccount SQS sensor capability #87
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
Multiaccount SQS sensor capability #87
Conversation
bb752e3 to
4db6b59
Compare
4db6b59 to
e06a9b2
Compare
blag
left a comment
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.
Looking mostly good, but needs some improvements.
9ce1319 to
314351d
Compare
blag
left a comment
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.
One last tweak to exception message, but I think this is looking good.
blag
left a comment
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.
Fixes for my own typos.
|
And renamed 'roles_arns' to 'roles'. |
blag
left a comment
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.
Thanks for seeing this through!
68d386e to
ae4a6f0
Compare
nmaludy
left a comment
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!
nmaludy
left a comment
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 two small changes.
Can you please add a CHANGELOG entry in: https://github.com/StackStorm-Exchange/stackstorm-aws/blob/master/CHANGES.md
And also bump the version in pack.yaml (https://github.com/StackStorm-Exchange/stackstorm-aws/blob/master/pack.yaml#L22) by 0.1 to be 1.3.0.
Please and thank you!
After these two things i'll gladly merge!
nmaludy
left a comment
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
Fixes #83