codeintel: make usage-range non-optional#64236
Conversation
✅ Cloud Controller GraphQL Compatability Test Resultsourcegraph/controller uses the GraphQL API to perform automation. The compatibility test has passed. Learn more from workflow logs. |
|
Changing |
| # Invariant: `range.path` == `blob.path`. The path is made available | ||
| # as part of this type for convenience. | ||
| # """ | ||
| usageRange: UsageRange |
There was a problem hiding this comment.
I think this signature will need to change:
UsageRange(context.Context) (UsageRangeResolver, error)
It needs to return a UsageRangeResolver now, similar to the other non-optional fields.
There was a problem hiding this comment.
On second glance, it seems a bit weird that we're allowed to have:
Provenance(context.Context) (codenav.CodeGraphDataProvenance, error)
even thought that field is non-optional. Maybe the point is that returning an error will keep bubbling it up until the first optional field? 🤔
| const repoGroups: RepoGroup[] = [] | ||
|
|
||
| for (const usage of usages) { | ||
| const repo = usage.usageRange!.repository |
There was a problem hiding this comment.
There are also similar usageRange! accesses in ExplorePanelFileUsages.svelte
There was a problem hiding this comment.
I think I'd need to find the place the type for usageRange is defined, and then I'd get lint warnings for unnecessary !s everywhere.
There was a problem hiding this comment.
If you regenerate graphql-operations.ts, then you should get that.
There was a problem hiding this comment.
I ran all of the sg gen commands and none seem to have worked. I'll back out the .svelte changes and wait for someone from the search team to take a look.
|
@kritzcreek I'm happy to follow up with the changes in a second PR. It doesn't look like this PR will actually change how the client needs to call the API, it just lets me get rid of a bunch of non-null assertions Thanks for the update! |
Closes https://linear.app/sourcegraph/issue/GRAPH-727/make-range-for-usagerange-non-optional
@camdencheek I'm not sure what changes the client requires after merging this PR. Would you prefer I merge here and you make a separate one, or would you rather push the changes onto this branch and merge in one go?
Test plan
Tests continue to pass