This repository was archived by the owner on Sep 30, 2024. It is now read-only.
graphqlbackend: Add changelistURL to GitBlob#53605
Merged
indradhanush merged 2 commits intomainfrom Jun 16, 2023
Merged
Conversation
indradhanush
added
perforce
graphqlbackend
perforce-changelists
4 tasks
Contributor
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 5089e69...6379006.
|
Contributor
This was referenced Jun 16, 2023
sashaostrikov
approved these changes
Jun 16, 2023
| } | ||
|
|
||
| func (r *GitTreeEntryResolver) ChangelistURL(ctx context.Context) (*string, error) { | ||
| repo := r.Repository() |
Contributor
There was a problem hiding this comment.
just a sanity check: we won't fetch repo here for the second/n-th time, right?
Contributor
Author
There was a problem hiding this comment.
Nope. It returns the r.commit.repoResolver which is already set as a backlink in the GitTreeEntryResolver which is set when the resolver is initialised.
Good check though!
| } | ||
|
|
||
| func (r *PerforceChangelistResolver) cidURL() *url.URL { | ||
| // Do not mutate the URL on the RepoMatch object. |
Contributor
There was a problem hiding this comment.
I think this comment may be misleading since RepoMatch doesn't contain the URL itself, it is constructed from other properties and returned as a new object which we can totally mutate.
Contributor
Author
There was a problem hiding this comment.
Ah - I agree. I will fix in a follow up. Going ahead with the merge because branch cut in ~8 mins. :P
coury-clark
pushed a commit
that referenced
this pull request
Jun 21, 2023
…st view page (#53668) Stacked on #53605. Part of #40330. ## Test plan 🎥 https://www.loom.com/share/a516e86ffc4341f7a7bfd83c1808c7bd?sid=d6d1d5fe-5842-458e-8078-4fb3d4e9d0c1 <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> <br> Backport 65345a7 from #53608 Co-authored-by: Indradhanush Gupta <indradhanush.gupta@gmail.com>
ErikaRS
pushed a commit
that referenced
this pull request
Jun 22, 2023
Part of #40330. ## Why We want to be able to generate permalinks for files in a perforce depot ## What - Add the `changelistURL` property to the `File2` interface - Add the `changelistURL` property to all the types that implement the `File2` interface - Implement the `ChangelistURL` API and return a changelist URL for the `GitBlob` type - Return nil for all the other types that implement `File2` (existing behaviour for these types, for example `url` is also not implemented on them) Following existing conventions. Ideally we should have a top level `urls` node that embeds all the different type of urls instead of having them on the top level definition in the `File2` interface, but that is a major API change that we're not ready for
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #40330.
Why
We want to be able to generate permalinks for files in a perforce depot
What
changelistURLproperty to theFile2interfacechangelistURLproperty to all the types that implement theFile2interfaceChangelistURLAPI and return a changelist URL for theGitBlobtypeFile2(existing behaviour for these types, for exampleurlis also not implemented on them)Following existing conventions. Ideally we should have a top level
urlsnode that embeds all the different type of urls instead of having them on the top level definition in theFile2interface, but that is a major API change that we're not ready forTest plan