Skip to content

Conversation

@jhchabran
Copy link
Contributor

In the context of scaletesting, it's pretty useful to be able to add code hosts on the fly, while toying with a test instance. This PR adds such a command.

Test plan

Manually tested.

In the context of scaletesting, it's pretty useful to be able to add
code hosts on the fly, while toying with a test instance. This PR adds
such a command.
Copy link
Contributor

@mrnugget mrnugget left a comment

Choose a reason for hiding this comment

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

I know the mutation is called addExternalService, but all the other CRUD commands in src-cli use create as the verb: src users create, src orgs create, ... I think create would be better.

Comment on lines 50 to 54
_, err := lookupExternalService(ctx, client, "", *nameFlag)
if err != errServiceNotFound {
return errors.New("service already exists")
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to do that lookup or can't you be optimistic and have the server fail if there's a problem with a duplicate name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I naively assumed we didn't want duplicate and this wasn't supported. Fixed as we actually support this.

}
return err
} else if ok {
fmt.Println("External service created:", *nameFlag)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fmt.Println("External service created:", *nameFlag)
fmt.Println("External service created: ", *nameFlag)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is going to print two spaces, with your suggestion:

~/work/src-cli US jh/extsvc-add $ go run ./cmd/src extsvc create -kind GITHUB -name testjhf config.json
External service created:  testjhf

@jhchabran jhchabran merged commit 23cab1f into main Oct 10, 2022
@jhchabran jhchabran deleted the jh/extsvc-add branch October 10, 2022 15:20
scjohns pushed a commit that referenced this pull request Apr 24, 2023
* Add a add subcommand to extsvc

In the context of scaletesting, it's pretty useful to be able to add
code hosts on the fly, while toying with a test instance. This PR adds
such a command.

* fixup

* Small fixes
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.

4 participants