feat(cli): enable customized env resources while initialization#1243
Conversation
68807a4 to
438f3f9
Compare
efekarakus
left a comment
There was a problem hiding this comment.
Awesome, I can't wait for this to be merged :D
efekarakus
left a comment
There was a problem hiding this comment.
Looks good! Only tiny things
| if o.NoCustomResources { | ||
| return nil | ||
| } | ||
| adjustOrImport, err := o.prompt.SelectOne( |
There was a problem hiding this comment.
We should try to use the WithFinalMessage option while prompting to provide a consistent experience but, in this situation it's a bit tricky since the options are a bit verbose. Just something to keep in mind :)
| } | ||
| _, vpcCIDR, err := net.ParseCIDR(vpcCIDRString) | ||
| if err != nil { | ||
| return fmt.Errorf("parse VPC CIDR: %w", err) |
There was a problem hiding this comment.
should we include the input in the error msg? or is that returned by the std library already?
There was a problem hiding this comment.
i believe the input is included
There was a problem hiding this comment.
Actually just carefully looked at it, this error ideally should never be triggered because we already have validation for the input. this is just because our Get method only return a string instead of IPNet method (we could make this change in the future), so that we need to parse the string to IPNet again...
kohidave
left a comment
There was a problem hiding this comment.
Looks great! One follow up we can do is include the name of the Subnets/VPCs - not just the IDs.
| } | ||
|
|
||
| // ListVPC returns IDs of all VPCs. | ||
| func (c *EC2) ListVPC() ([]string, error) { |
There was a problem hiding this comment.
One thing that might be nice to do is return a struct with the VPC ID and the value of the "Name" tag if it exists. This is what the console does.
There was a problem hiding this comment.
Looks great! One follow up we can do is include the name of the Subnets/VPCs - not just the IDs.
i'll cut an issue for it!
|
|
||
| // ListVPCSubnets lists all subnets given a VPC ID. | ||
| func (c *EC2) ListVPCSubnets(vpcID string) (*Subnets, error) { | ||
| func (c *EC2) ListVPCSubnets(vpcID string, opts ...ListVPCSubnetsOpts) ([]string, error) { |
There was a problem hiding this comment.
Same advice regarding the "Name" tag - so that customers can see the names if they exist.
Last part of #1192. Update
env initUI to enable create customized env resources in an interactive way.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.