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

Support following single-file history across renames in GraphQL API#45882

Merged
sqs merged 1 commit into
mainfrom
sqs/follow-renames
Dec 24, 2022
Merged

Support following single-file history across renames in GraphQL API#45882
sqs merged 1 commit into
mainfrom
sqs/follow-renames

Conversation

@sqs

@sqs sqs commented Dec 21, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Adds GitCommit.ancestors(follow: Boolean) to the GraphQL API to track a single file's commit history across renames.

Helps with #4383. See https://github.com/sourcegraph/sourcegraph/pull/45882#issuecomment-1364482590 for the additional backend, API, and UI work needed to close that issue.

Test plan

Call ancestors in the GraphQL API with a path and follow: true. Ensure it shows commits before a rename.

@sqs sqs requested a review from a team December 21, 2022 06:29
@cla-bot cla-bot Bot added the cla-signed label Dec 21, 2022
@sourcegraph-bot

sourcegraph-bot commented Dec 21, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 990e3be...564a35e.

Notify File(s)
@indradhanush internal/gitserver/commands.go
internal/gitserver/commands_test.go
@ryanslade internal/gitserver/commands.go
internal/gitserver/commands_test.go
@sashaostrikov internal/gitserver/commands.go
internal/gitserver/commands_test.go

@sg-e2e-regression-test-bob

sg-e2e-regression-test-bob commented Dec 21, 2022 •
edited
Loading

Copy link
Copy Markdown

Bundle size report 📦

Initial size Total size Async size Modules
0.00% (0.00 kb) 0.00% (+0.15 kb) 0.00% (+0.15 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits 8b56a9e and 990e3be or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

sashaostrikov
sashaostrikov reviewed Dec 21, 2022
View reviewed changes

@sashaostrikov sashaostrikov 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.

LGTM, except panicking, left a comment about that

Comment thread internal/gitserver/commands.go Outdated
philipp-spiess
philipp-spiess approved these changes Dec 22, 2022
View reviewed changes

@philipp-spiess philipp-spiess 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 improvement! Let's remove the panic and :shipit:

@sqs sqs force-pushed the sqs/follow-renames branch from a845ff2 to 8b56a9e Compare December 24, 2022 07:28
@sqs

sqs commented Dec 24, 2022

Copy link
Copy Markdown
Member Author

This actually needs more work. The reason is that in the panel, the file icon (pictured below) needs to link to the previous filename, not the current filename. That means we need to track the filename over history and pass it back via the API, which means we need something different than just a general GitCommitConnection (like a FileHistory type or FileCommitConnection type).

image

I am going to merge this PR with just the new GraphQL API (which is semi-valuable by itself) and leave the issue for the UI functionality open (pending the addition of that FileHistory/FileCommitConnection type).

@sqs sqs force-pushed the sqs/follow-renames branch from 8b56a9e to ba355fd Compare December 24, 2022 08:01
@sqs sqs changed the title follow single-file history across renames Support following single-file history across renames in GraphQL API Dec 24, 2022
@sqs sqs force-pushed the sqs/follow-renames branch from ba355fd to 564a35e Compare December 24, 2022 08:04
@sqs sqs enabled auto-merge (squash) December 24, 2022 08:04
@sqs sqs merged commit ac60779 into main Dec 24, 2022
@sqs sqs deleted the sqs/follow-renames branch December 24, 2022 08:19
@sqs sqs mentioned this pull request Dec 26, 2022
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@sashaostrikov sashaostrikov sashaostrikov left review comments

@philipp-spiess philipp-spiess philipp-spiess approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sqs @sourcegraph-bot @sg-e2e-regression-test-bob @philipp-spiess @sashaostrikov