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

codeintel: fetches diffs from gitserver in batches#64025

Merged
kritzcreek merged 2 commits into
mainfrom
christoph/batch-fetch-diff
Jul 24, 2024
Merged

codeintel: fetches diffs from gitserver in batches#64025
kritzcreek merged 2 commits into
mainfrom
christoph/batch-fetch-diff

Conversation

@kritzcreek

Copy link
Copy Markdown
Contributor

Closes https://linear.app/sourcegraph/issue/GRAPH-769/fetch-diffs-in-batches

Test plan

Covered by existing tests (noticeable because tests need updates as we now use the path out of the returned diff)

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

@varungandhi-src varungandhi-src 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.

Nice work!

Comment thread internal/codeintel/codenav/gittree_translator.go Outdated
Comment thread internal/codeintel/codenav/gittree_translator.go Outdated
Comment thread internal/codeintel/codenav/gittree_translator.go Outdated
Comment thread internal/codeintel/codenav/gittree_translator.go Outdated

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.

What's the reason behind changing this test case? Is this related to your point about "noticeable because tests need updates as we now use the path out of the returned diff"/I don't quite get that?

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.

Yes, exactly. The file diff iterator will only contain entries for files that have diffs, which means we can't rely on it giving us results we can zip with the input paths.
That means we have to rely on the file path contained in the diff, which means that in these tests we need to make sure to line up these file paths.

As we only searched for one path at a time previously the code would just handle the "empty iterator" as inputPath -> []diff{}.

@kritzcreek kritzcreek force-pushed the christoph/batch-fetch-diff branch from 17fdad2 to 5c778ca Compare July 24, 2024 06:01
@kritzcreek kritzcreek enabled auto-merge (squash) July 24, 2024 06:01
@kritzcreek kritzcreek merged commit eefcc7e into main Jul 24, 2024
@kritzcreek kritzcreek deleted the christoph/batch-fetch-diff branch July 24, 2024 06:12
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