Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Allow selection of fork/parent pull requests. #1021
Merged
Conversation
Previously if the user was working on a fork, the PRs for the fork would be shown rather than the PRs for the parent repository. Change the default to show PRs for the parent repository with a (very ugly!) checkbox to switch to showing the PRs for the fork.
Recreate the `PullRequests` `TrackingCollection` when switching to/from showing PRs for a fork. Also use the repository owner name in `ModelService.GetPullRequests` instead of the current username.
Take into account whether the PR is from a fork or not in `PullRequestDetailViewModel`.
Previously we simply did a check to see if `Head.Owner` != `Base.Owner` but that didn't work when we were _in_ a fork and looking at a PR in the parent. In that case both head and base are in a different repository.
This is better than the ugly checkbox there was before, but it's easy for it to get too long and wrap, which looks bad.
grokys
added a commit
that referenced
this pull request
Jun 29, 2017
In #1021 we require the repository parent (in the case of forked repositories) - this data is returned from the API but wasn't getting stored in our cache, meaning that we have invalid data when data is returned from the cache as opposed to from the API. This adds a `cacheVersion` to host caches. This version is checked when the host cache is first loaded, and if it doesn't match the expected version, the cache is invalidated. In this way each time we make a change as required by #1021 we can bump `HostCacheFactory.CacheVersion` to ensure the cache won't be returning invalid data.
`Repository.Owner` is now being used by `ModelService.GetPullRequest`.
|
@grokys Tried testing this PR, but the PR page in the github window just hangs, with "Unable to complete read operation after process has exited" in the Output window. Not sure how to get around this.
|
Using the same one with different contents means that one cannot downgrade the assembly as the older version throws on the new format.
…-requests Conflicts: src/GitHub.InlineReviews/Services/PullRequestSession.cs
|
@tierninho could you try now? |
|
@grokys same result, still hangs. |
|
Strange. This PR has been working well for me. |
|
@grokys and I got this working on my machine after tweaking my setup |
|
Works on my machine! |
Conflicts: src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs
This will ensure that required commits/blobs are available.
When merge-base can't be found, fetch the PR base and head from the correct repo URLs.
src/GitHub.App/Services/GitClient.cs
Outdated
| { | ||
| var mergeBase = GetMergeBase(repo, baseSha, headSha); | ||
| if (mergeBase == null) | ||
| { | ||
| var pullHeadRef = $"refs/pull/{pullNumber}/head"; | ||
| await Fetch(repo, remoteName, baseRef, pullHeadRef); | ||
| // TODO: Optimize and error check. |
jcansdale
Jul 14, 2017
Collaborator
I need to fix this.
I need to fix this.
Build was failing because this was excluded.
We no longer know whether the local repo will be on base or head end of the PR. Fetch using repo URL and PR base/head ref.
src/GitHub.App/Services/GitClient.cs
Outdated
| @@ -394,6 +401,20 @@ public Task<bool> IsHeadPushed(IRepository repo) | |||
| }); | |||
| } | |||
|
|
|||
| Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs) | |||
| { | |||
| var tempRemoteName = $"{cloneUrl.Owner}-{Guid.NewGuid()}"; | |||
jcansdale
Jul 17, 2017
Collaborator
This should only create temporary remotes when necessary (when cloneUrl != origin).
This should only create temporary remotes when necessary (when cloneUrl != origin).
There is a good change the PR base or head comes from `origin`. Use this remote when possible.
…-requests Conflicts: src/GitHub.App/Models/RemoteRepositoryModel.cs src/GitHub.App/Services/GitClient.cs src/GitHub.App/ViewModels/PullRequestListViewModel.cs
This was removed by a bad merge in 287ca67
|
Didn't actually mean to block this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When working on a fork, previously the PR list would show PRs for that fork, which is often not what is wanted. This PR adds a selector dropdown to select the repository from which to display the pull requests, defaulting to the parent repository if the repository is a fork. If the repository is not a fork, the selector will be hidden.
This exposes a similar problem to #734 in that there quickly ends up being no room along the top row of filters for the repository name, meaning it wraps to a new line, causing ugly. We need to think of a new way to display filters for this and #734. cc: @donokuda
Depends on #1004, #1023
Fixes #863