Skip to content
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

feat: create AWS account #45

Merged
merged 10 commits into from
Aug 3, 2022
Merged

feat: create AWS account #45

merged 10 commits into from
Aug 3, 2022

Conversation

domenicsim1
Copy link
Collaborator

No description provided.

@domenicsim1 domenicsim1 requested a review from a team August 2, 2022 23:51
pkg/cmd/account/aws/create/create.go Outdated Show resolved Hide resolved
pkg/cmd/account/helper/helper.go Outdated Show resolved Hide resolved
for _, match := range allMatches {
if envName == match.Name {
envIds = append(envIds, match.ID)
continue loop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to think pretty hard to work out the logic here...

Perhaps it would be simpler just to do an ordinary loop with a break (and no continue-outer) and then just put an if statement at the end which says if(len(envIds) == 0) { envIds = envIds.Append(envName) }

if err != nil {
f.Spinner().Stop()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, this spinner is getting a bit awkward, I wonder if we can do something to help with defer etc. I have no solutions at the moment, but it's something to think about in the background

if err != nil {
return nil, err
}
itemIds := make([]string, 0, len(items))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use util.SliceTransform here.

I'm unsure if we should though? How good is go at inlining lambdas?

pkg/surveyext/editor.go Show resolved Hide resolved
@domenicsim1 domenicsim1 merged commit 9dd0d64 into main Aug 3, 2022
@domenicsim1 domenicsim1 deleted the feat/create-account branch August 3, 2022 00:48
This was referenced Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants