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

chore(codeintel): Differentiate between paths relative to upload root vs repo root#63437

Merged
varungandhi-src merged 8 commits into
mainfrom
vg/separate-paths
Jun 24, 2024
Merged

chore(codeintel): Differentiate between paths relative to upload root vs repo root#63437
varungandhi-src merged 8 commits into
mainfrom
vg/separate-paths

Conversation

@varungandhi-src

@varungandhi-src varungandhi-src commented Jun 24, 2024

Copy link
Copy Markdown
Contributor

Should hopefully make the code easier to follow due to the mixture of
different kinds of paths in different places.

Test plan

TODO:

  • Update existing tests
  • Locally upload scip-go index at different roots for sg monorepo and check that
    codenav is working OK - checked for lib subdir

Changelog

@cla-bot cla-bot Bot added the cla-signed label Jun 24, 2024
@github-actions github-actions Bot added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform labels Jun 24, 2024
… vs repo root

This code identifies a bug in the SCIPDocument method which should
be handling a conversion from the RepoRootRelativePath to an
UploadRootRelativePath
@varungandhi-src varungandhi-src changed the title WIP: Differentiate between paths relative to upload root vs repo root chore(codeintel): Differentiate between paths relative to upload root vs repo root Jun 24, 2024

@kritzcreek kritzcreek left a comment

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 think it's a good idea to un-stringify these.

Naming is always hard, but maybe we can find something a little shorter to reduce the clutter? How about RepoRelPath & UploadRelPath?

Comment thread internal/codeintel/codenav/gittree_translator_test.go Outdated
Comment thread internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go Outdated
Comment thread internal/codeintel/codenav/transport/graphql/root_resolver_test.go Outdated
Comment thread internal/codeintel/core/paths.go Outdated
@varungandhi-src varungandhi-src force-pushed the vg/separate-paths branch 2 times, most recently from 7a80a63 to 5b16f26 Compare June 24, 2024 11:16
@varungandhi-src varungandhi-src marked this pull request as ready for review June 24, 2024 11:20
@varungandhi-src varungandhi-src merged commit 34b54c9 into main Jun 24, 2024
@varungandhi-src varungandhi-src deleted the vg/separate-paths branch June 24, 2024 12:27
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