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

batches: Remote Mount #2 - GraphQL#40809

Merged
Piszmog merged 86 commits into
mainfrom
rc/remote-mount-graphql
Sep 27, 2022
Merged

batches: Remote Mount #2 - GraphQL#40809
Piszmog merged 86 commits into
mainfrom
rc/remote-mount-graphql

Conversation

@Piszmog

@Piszmog Piszmog commented Aug 24, 2022

Copy link
Copy Markdown
Contributor

Part of #31792.

Previous part: #40795

Added a GraphQL type for Batch Spec Mounts. This allows metadata to be retrieved about the files.

Note: the content of the files will be retrieved from a HTTP endpoint in a later PR.

Test plan

Add Go Unit Tests

@Piszmog Piszmog requested a review from a team August 24, 2022 16:28
@cla-bot cla-bot Bot added the cla-signed label Aug 24, 2022
@sourcegraph-bot

sourcegraph-bot commented Aug 24, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff d1d3f31...d9b9bf7.

Notify File(s)
@courier-new cmd/frontend/graphqlbackend/batches.go
@eseliger cmd/frontend/graphqlbackend/batches.go

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the diff for store stuff is duplicative in here, has it landed yet? Otherwise, maybe stack the two PRs?

@Piszmog

Piszmog commented Aug 24, 2022

Copy link
Copy Markdown
Contributor Author

Yea this branch is off of the first one

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approve to unblock, one question about the naming and inheritance of File2 that we might want to address.

Comment thread cmd/frontend/graphqlbackend/batches.go Outdated
Comment thread cmd/frontend/graphqlbackend/batches.go Outdated
Comment thread cmd/frontend/graphqlbackend/batches.graphql Outdated
Comment thread enterprise/cmd/frontend/internal/batches/resolvers/batch_spec.go Outdated
Comment thread enterprise/cmd/frontend/internal/batches/resolvers/batch_spec.go Outdated
@Piszmog Piszmog changed the base branch from main to rc/remote-mount-db August 24, 2022 17:41
@Piszmog

Piszmog commented Aug 24, 2022

Copy link
Copy Markdown
Contributor Author

@eseliger I updated the base to make the diffs easier to see and more targeted

@eseliger

Copy link
Copy Markdown
Member

Thanks! Good work splitting it up :)

@LawnGnome LawnGnome 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 might have made the opposite choice on File2 inheritance, but I trust you and Erik to get this right. Definitely no further changes required.

Comment thread enterprise/cmd/frontend/internal/batches/resolvers/batch_spec.go Outdated
Comment thread enterprise/cmd/frontend/internal/batches/resolvers/workspace_file.go Outdated
Comment thread cmd/frontend/graphqlbackend/batches.graphql Outdated
Comment thread enterprise/cmd/frontend/internal/batches/resolvers/batch_spec_test.go Outdated
Base automatically changed from rc/remote-mount-db to main September 27, 2022 13:26
@Piszmog Piszmog merged commit d2cdb05 into main Sep 27, 2022
@Piszmog Piszmog deleted the rc/remote-mount-graphql branch September 27, 2022 14:56
sashaostrikov pushed a commit that referenced this pull request Sep 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants