-
Notifications
You must be signed in to change notification settings - Fork 22
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 tags and boot script #35
Conversation
TestingInteractive mode CX
|
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. Thanks for the changes.
Left a few minor comments.
pkg/question/question.go
Outdated
data = append(data, []string{cli.ResourceUserTags, strings.Join(simpleConfig.UserTags, "\n")}) | ||
var tags []string | ||
for k := range simpleConfig.UserTags { | ||
tags = append(tags, fmt.Sprintf("%s | %s", k, simpleConfig.UserTags[k])) |
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.
format below tag1|val1
without spaces. May be get rid of extra spaces for clarity?
Or how about adding a header to the tags slice here like tags
var tags = []string{"KEY | VALUE"}
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.
- Removing the spaces from
tag1|val1
- Your suggestion is definitely clearer, but the way table is implemented and formatted now it'd be difficult to get such a view today
- In the mean time, I'll format it in the final confirmation to be cleaner and revisit when/if we refactor the table logic:
+--------------------------------------+--------------------------+
| Tag Specification(key|value) | taggd|Bryan |
| | revd:|pdk |
+--------------------------------------+--------------------------+
Issue #, if available:
Description of changes:
--boot-script
or-b
--tags
auto-termination-time
ANDboot-script
, the shutdown command will be prepended to the user's scriptTesting:
make unit-test
./build/simple-ec2 launch -i
>cat test-boot
:✅ user script took effect & instance shutdown as expected
./build/simple-ec2 launch -b /Users/crtbry/workplace/go/src/simpleEc2_brycahta_fork_ws/aws-simple-ec2-cli/test-boot-2 --tags createdBy:Bryan,reviewedBy:PdK --region us-east-1 -a 3
>cat test-boot-2
:✅ user script took effect & auto-termination-timer was applied & instance shutdown as expected
./build/simple-ec2 launch -h
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.