Skip to content

internal/batches: include the url field in repositories#625

Merged
LawnGnome merged 1 commit into
mainfrom
aharvey/restore-batch-repos
Oct 6, 2021
Merged

internal/batches: include the url field in repositories#625
LawnGnome merged 1 commit into
mainfrom
aharvey/restore-batch-repos

Conversation

@LawnGnome

Copy link
Copy Markdown
Contributor

Removing this in #598 broke the src batch repos command, which used the url field when rendering the link to the repo in Sourcegraph.

Removing this in #598 broke the `src batch repos` command, which used
the `url` field when rendering the link to the repo in Sourcegraph.
@LawnGnome LawnGnome added bug Something isn't working team/code-search labels Oct 6, 2021
@LawnGnome LawnGnome requested a review from a team October 6, 2021 01:43
@LawnGnome LawnGnome self-assigned this Oct 6, 2021
@eseliger

eseliger commented Oct 6, 2021

Copy link
Copy Markdown
Member

@mrnugget should we include this in the exec json files as well then? I removed it before (for one, because I thought it was unused) because I feared that at some point we use this field on the struct because "it's there", but we didn't pass it down in the SSBC path.

@mrnugget

mrnugget commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

Which files? Do you mean the JSON log line output?

@eseliger

eseliger commented Oct 6, 2021

Copy link
Copy Markdown
Member

The json input file for the batch exec command.

@LawnGnome LawnGnome merged commit 5d44100 into main Oct 6, 2021
@LawnGnome LawnGnome deleted the aharvey/restore-batch-repos branch October 6, 2021 17:20
@mrnugget

mrnugget commented Oct 7, 2021

Copy link
Copy Markdown
Contributor

It would probably make sense, just to be consistent.

@mrnugget

mrnugget commented Oct 8, 2021

Copy link
Copy Markdown
Contributor

Hmm, the url returned in GraphQL is basically just the name of the repository:

https://github.com/sourcegraph/sourcegraph/blob/f0e6c5298df55ff77316c49b834d2805ce735236/cmd/frontend/graphqlbackend/repository.go#L252-L254

https://github.com/sourcegraph/sourcegraph/blob/f0e6c5298df55ff77316c49b834d2805ce735236/internal/search/result/repo.go#L43-L49

I wonder whether it wouldn't be confusing if we tried to reconstruct that method just for the sake of passing it down to src batch exec, where it'll be unused.

scjohns pushed a commit that referenced this pull request Apr 24, 2023
Removing this in #598 broke the `src batch repos` command, which used
the `url` field when rendering the link to the repo in Sourcegraph.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working team/code-search

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants