Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

chore: Rename URI -> DocumentPath#63979

Merged
varungandhi-src merged 1 commit into
mainfrom
vg/rename-3
Jul 22, 2024
Merged

chore: Rename URI -> DocumentPath#63979
varungandhi-src merged 1 commit into
mainfrom
vg/rename-3

Conversation

@varungandhi-src

Copy link
Copy Markdown
Contributor

If you look at the code for bulkMonikerResultsQuery and minimalBulkMonikerResultsQuery,
you'll see that the last returned value is the document_path. The value returned
was directly used an UploadRootRelPath elsewhere. So it makes sense to rename
the field from URI to DocumentPath.

Test plan

Covered by existing tests

@cla-bot cla-bot Bot added the cla-signed label Jul 22, 2024
@github-actions github-actions Bot added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform labels Jul 22, 2024

locations = append(locations, precise.LocationData{
URI: uri,
DocumentPath: uri,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a bit weird to use DocumentPath: uri here, but we don't care about LSIF conversion. Let's look at user stats to see if people are still uploading LSIF; otherwise let's deprecate & delete the code.

I'll file a follow-up issue for this tomorrow.

@kritzcreek kritzcreek Jul 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought we made it so people can't actually upload lsif anymore, and we convert it to scip at the src-cli level?

EDIT: Oh, this is the lib code that ends up getting imported from src-cli.

@varungandhi-src varungandhi-src enabled auto-merge (squash) July 22, 2024 12:52
@varungandhi-src varungandhi-src merged commit a5dad3b into main Jul 22, 2024
@varungandhi-src varungandhi-src deleted the vg/rename-3 branch July 22, 2024 13:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants