Add --role-arn option for remove sub command. #98
Conversation
gouglhupf
force-pushed
the
add-role-arn-rm-command
branch
from
4d9f749 to
22bcfe5
Compare
gouglhupf
force-pushed
the
add-role-arn-rm-command
branch
from
22bcfe5 to
6525fa6
Compare
| @@ -187,7 +187,7 @@ YAML: | |||
| } | |||
|
|
|||
| if !stackExists { | |||
| err = cfn.DeleteStack(stackName) | |||
| err = cfn.DeleteStack(stackName, "") | |||
There was a problem hiding this comment.
Should we also apply the role arn here?
There was a problem hiding this comment.
Good question. As I understand, we created the stack from scratch and deleting it if the user doesn't deploy the change set. If the user passes the role arn flag, the stack will already have the role set. If no role is passed to the API, it will reuse the correct role. Note that the stack is empty, so CloudFormation might not assume the role as no resources are being touched (no sure about that).
In my view, adding the role ARN flag is a matter of code intelligibility rather than functionality.
What do you think?
Thanks for the reviews!
There was a problem hiding this comment.
As long as it won't fail, we don't need to add it.
Issue #, if available: -
Description of changes: Add --role-arn option for remove sub command.
The PR #81 added this flag to the deploy command. On occasion we needed this flag for the remove (delete) command too.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.