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

GraphQL ExternalLink should expose kind: ExternalServiceKind (enum) instead of serviceType: String #14979

Closed
felixfbecker opened this issue Oct 22, 2020 · 1 comment · Fixed by #16465
Assignees
Labels
api Sourcegraph GraphQL API estimate/2d good first issue Good for newcomers
Milestone

Comments

@felixfbecker
Copy link
Contributor

ExternalLink in GraphQL currently only exposes the serviceType as a string. This has caused a lot of headache in the frontend as we need to enumerate it to show appropiate icons, show the right browser extension install alerts, etc. Every time I have to go dig up the possible values in the backend, which is non-trivial because we have many properties named serviceType that are different things.

In a recent case this caused our alert logic to not be aware that external services like AWS code commit exist (which we don't support in the browser extension), resulting in us showing alerts to install the browser extension despite that code host not being supported. We could have avoided that mistake with more diligence of course, but I'm convinced that would not have happened had the property been an enum, so that the possible values would have been easily visible in TypeScript types/hovers/autocompletion.

Expected:

    kind: ExternalServiceKind
@felixfbecker felixfbecker added api Sourcegraph GraphQL API team/cloud labels Oct 22, 2020
@github-actions
Copy link
Contributor

Heads up @tsenart - the "team/cloud" label was applied to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Sourcegraph GraphQL API estimate/2d good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants