-
Notifications
You must be signed in to change notification settings - Fork 147
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
[aws-json] Migrate SNS ListTopics #311
[aws-json] Migrate SNS ListTopics #311
Conversation
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.
Couple tiny things, otherwise looks great!
} | ||
|
||
arnList := make([]models.TopicArnResult, 0) | ||
|
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.
Optional: I kind of liked the log in here, though it could be debug
probably. Up to you though.
8cbd809
to
03e4e02
Compare
Ready to merge! |
bb4a35e
to
a91f51b
Compare
minor fixes add not implemented comment to sns model reset tests affecting new tests add debug message when listing sns topics remove merge remnants, minor fixups update listtopics test to handle new mock config
Looks like something is up with the unit tests on these now? If we can get that sorted I think they look good to go! |
a91f51b
to
71373a6
Compare
@Admiral-Piett Sorry, fixed the test on ListTopics/DeleteTopic! |
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.
Excellent! Thank you!
19dd37a
into
Admiral-Piett:feature-aws-json
No description provided.