Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit db89b97

Browse files
grokysgrokys
authored
Merge pull request #1221 from github/fixes/997-default-pr-message
Pre-fill PR title/description.
2 parents 5044714 + 0b03380 commit db89b97

File tree

9 files changed

+236
-13
lines changed

9 files changed

+236
-13
lines changed

‎src/GitHub.App/Services/GitClient.cs‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.ComponentModel.Composition;
34
using System.IO;
45
using System.Linq;
56
using System.Threading.Tasks;
67
using GitHub.Extensions;
8+
using GitHub.Models;
79
using GitHub.Primitives;
810
using LibGit2Sharp;
911
using GitHub.Logging;
@@ -38,7 +40,6 @@ public GitClient(IGitHubCredentialProvider credentialProvider)
3840
public Task Pull(IRepository repository)
3941
{
4042
Guard.ArgumentNotNull(repository, nameof(repository));
41-
4243
return Task.Factory.StartNew(() =>
4344
{
4445
var signature = repository.Config.BuildSignature(DateTimeOffset.UtcNow);
@@ -474,6 +475,39 @@ public Task<bool> IsHeadPushed(IRepository repo)
474475
});
475476
}
476477

478+
public Task<IReadOnlyList<CommitMessage>> GetMessagesForUniqueCommits(
479+
IRepository repo,
480+
string baseBranch,
481+
string compareBranch,
482+
int maxCommits)
483+
{
484+
return Task.Factory.StartNew(() =>
485+
{
486+
var baseCommit = repo.Lookup<Commit>(baseBranch);
487+
var compareCommit = repo.Lookup<Commit>(compareBranch);
488+
if (baseCommit == null || compareCommit == null)
489+
{
490+
var missingBranch = baseCommit == null ? baseBranch : compareBranch;
491+
throw new NotFoundException(missingBranch);
492+
}
493+
494+
var mergeCommit = repo.ObjectDatabase.FindMergeBase(baseCommit, compareCommit);
495+
var commitFilter = new CommitFilter
496+
{
497+
IncludeReachableFrom = baseCommit,
498+
ExcludeReachableFrom = mergeCommit,
499+
};
500+
501+
var commits = repo.Commits
502+
.QueryBy(commitFilter)
503+
.Take(maxCommits)
504+
.Select(c => new CommitMessage(c.Message))
505+
.ToList();
506+
507+
return (IReadOnlyList<CommitMessage>)commits;
508+
});
509+
}
510+
477511
static bool IsCanonical(string s)
478512
{
479513
Guard.ArgumentNotEmptyString(s, nameof(s));

‎src/GitHub.App/Services/PullRequestService.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,19 @@ public IObservable<string> GetPullRequestTemplate(ILocalRepositoryModel reposito
8787
});
8888
}
8989

90+
public IObservable<IReadOnlyList<CommitMessage>> GetMessagesForUniqueCommits(
91+
ILocalRepositoryModel repository,
92+
string baseBranch,
93+
string compareBranch,
94+
int maxCommits)
95+
{
96+
return Observable.Defer(() =>
97+
{
98+
var repo = gitService.GetRepository(repository.LocalPath);
99+
return gitClient.GetMessagesForUniqueCommits(repo, baseBranch, compareBranch, maxCommits).ToObservable();
100+
});
101+
}
102+
90103
public IObservable<bool> IsWorkingDirectoryClean(ILocalRepositoryModel repository)
91104
{
92105
var repo = gitService.GetRepository(repository.LocalPath);

‎src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs‎

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@
1818
using GitHub.Extensions;
1919
using System.Reactive.Disposables;
2020
using System.Reactive;
21-
using GitHub.Logging;
21+
using System.Threading.Tasks;
2222
using Serilog;
23+
using GitHub.Logging;
2324

2425
namespace GitHub.ViewModels
2526
{
@@ -72,8 +73,6 @@ public PullRequestCreationViewModel(IRepositoryHost repositoryHost, ILocalReposi
7273
});
7374

7475
SourceBranch = activeRepo.CurrentBranch;
75-
service.GetPullRequestTemplate(activeRepo)
76-
.Subscribe(x => Description = x ?? String.Empty, () => Description = Description ?? String.Empty);
7776

7877
this.WhenAnyValue(x => x.Branches)
7978
.WhereNotNull()
@@ -86,6 +85,31 @@ public PullRequestCreationViewModel(IRepositoryHost repositoryHost, ILocalReposi
8685

8786
SetupValidators();
8887

88+
var uniqueCommits = this.WhenAnyValue(
89+
x => x.SourceBranch,
90+
x => x.TargetBranch)
91+
.Where(x => x.Item1 != null && x.Item2 != null)
92+
.Select(branches =>
93+
{
94+
var baseBranch = branches.Item1.Name;
95+
var compareBranch = branches.Item2.Name;
96+
97+
// We only need to get max two commits for what we're trying to achieve here.
98+
// If there's no commits we want to block creation of the PR, if there's one commits
99+
// we wan't to use its commit message as the PR title/body and finally if there's more
100+
// than one we'll use the branch name for the title.
101+
return service.GetMessagesForUniqueCommits(activeRepo, baseBranch, compareBranch, maxCommits: 2)
102+
.Catch<IReadOnlyList<CommitMessage>, Exception>(ex =>
103+
{
104+
log.Warning(ex, "Could not load unique commits");
105+
return Observable.Empty<IReadOnlyList<CommitMessage>>();
106+
});
107+
})
108+
.Switch()
109+
.ObserveOn(RxApp.MainThreadScheduler)
110+
.Replay(1)
111+
.RefCount();
112+
89113
var whenAnyValidationResultChanges = this.WhenAny(
90114
x => x.TitleValidator.ValidationResult,
91115
x => x.BranchValidator.ValidationResult,
@@ -121,6 +145,37 @@ public PullRequestCreationViewModel(IRepositoryHost repositoryHost, ILocalReposi
121145
this.WhenAnyValue(x => x.Initialized, x => x.GitHubRepository, x => x.Description, x => x.IsExecuting)
122146
.Select(x => !(x.Item1 && x.Item2 != null && x.Item3 != null && !x.Item4))
123147
.Subscribe(x => IsBusy = x);
148+
149+
Observable.CombineLatest(
150+
this.WhenAnyValue(x => x.SourceBranch),
151+
uniqueCommits,
152+
service.GetPullRequestTemplate(activeRepo).DefaultIfEmpty(string.Empty),
153+
(compare, commits, template) => new { compare, commits, template })
154+
.Subscribe(x =>
155+
{
156+
var prTitle = string.Empty;
157+
var prDescription = string.Empty;
158+
159+
if (x.commits.Count == 1)
160+
{
161+
prTitle = x.commits[0].Summary;
162+
prDescription = x.commits[0].Details;
163+
}
164+
else
165+
{
166+
prTitle = x.compare.Name.Humanize();
167+
}
168+
169+
if (!string.IsNullOrWhiteSpace(x.template))
170+
{
171+
if (!string.IsNullOrEmpty(prDescription))
172+
prDescription += "\n\n";
173+
prDescription += x.template;
174+
}
175+
176+
PRTitle = prTitle;
177+
Description = prDescription;
178+
});
124179
}
125180

126181
public override void Initialize(ViewWithData data = null)

‎src/GitHub.Exports.Reactive/Services/IGitClient.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
using System.Threading.Tasks;
33
using LibGit2Sharp;
44
using GitHub.Primitives;
5+
using System.Collections.Generic;
6+
using GitHub.Models;
57

68
namespace GitHub.Services
79
{
@@ -204,5 +206,21 @@ public interface IGitClient
204206
/// <param name="repository">The repository.</param>
205207
/// <returns></returns>
206208
Task<bool> IsHeadPushed(IRepository repo);
209+
210+
/// <summary>
211+
/// Gets the unique commits from <paramref name="compareBranch"/> to the merge base of
212+
/// <paramref name="baseBranch"/> and <paramref name="compareBranch"/> and returns their
213+
/// commit messages.
214+
/// </summary>
215+
/// <param name="repository">The repository.</param>
216+
/// <param name="baseBranch">The base branch to find a merge base with.</param>
217+
/// <param name="compareBranch">The compare branch to find a merge base with.</param>
218+
/// <param name="maxCommits">The maximum number of commits to return.</param>
219+
/// <returns>An enumerable of unique commits from the merge base to the compareBranch.</returns>
220+
Task<IReadOnlyList<CommitMessage>> GetMessagesForUniqueCommits(
221+
IRepository repo,
222+
string baseBranch,
223+
string compareBranch,
224+
int maxCommits);
207225
}
208226
}

‎src/GitHub.Exports.Reactive/Services/IPullRequestService.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Reactive;
34
using System.Text;
5+
using System.Threading.Tasks;
46
using GitHub.Models;
57
using LibGit2Sharp;
68
using Octokit;
@@ -156,5 +158,21 @@ IObservable<string> ExtractFile(
156158
IObservable<Unit> RemoveUnusedRemotes(ILocalRepositoryModel repository);
157159

158160
IObservable<string> GetPullRequestTemplate(ILocalRepositoryModel repository);
161+
162+
/// <summary>
163+
/// Gets the unique commits from <paramref name="compareBranch"/> to the merge base of
164+
/// <paramref name="baseBranch"/> and <paramref name="compareBranch"/> and returns their
165+
/// commit messages.
166+
/// </summary>
167+
/// <param name="repository">The repository.</param>
168+
/// <param name="baseBranch">The base branch to find a merge base with.</param>
169+
/// <param name="compareBranch">The compare branch to find a merge base with.</param>
170+
/// <param name="maxCommits">The maximum number of commits to return.</param>
171+
/// <returns>An enumerable of unique commits from the merge base to the compareBranch.</returns>
172+
IObservable<IReadOnlyList<CommitMessage>> GetMessagesForUniqueCommits(
173+
ILocalRepositoryModel repository,
174+
string baseBranch,
175+
string compareBranch,
176+
int maxCommits);
159177
}
160178
}

‎src/GitHub.Exports/GitHub.Exports.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@
139139
<ItemGroup>
140140
<Compile Include="Exports\ExportForProcess.cs" />
141141
<Compile Include="GitHubLogicException.cs" />
142+
<Compile Include="Models\CommitMessage.cs" />
142143
<Compile Include="Models\DiffChangeType.cs" />
143144
<Compile Include="Models\DiffChunk.cs" />
144145
<Compile Include="Models\DiffLine.cs" />

‎src/GitHub.Exports/Models/CommitMessage.cs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
using System;
2+
using System.Linq;
3+
4+
namespace GitHub.Models
5+
{
6+
public class CommitMessage : IEquatable<CommitMessage>
7+
{
8+
public string Summary { get; private set; }
9+
public string Details { get; private set; }
10+
public string FullMessage { get; private set; }
11+
12+
/// <summary>
13+
/// This is for mocking porpoises.
14+
/// http://cl.ly/image/0q2A2W0U3O2t
15+
/// </summary>
16+
public CommitMessage() { }
17+
18+
public CommitMessage(string fullMessage)
19+
{
20+
if (string.IsNullOrEmpty(fullMessage)) return;
21+
22+
var lines = fullMessage.Replace("\r\n", "\n").Split('\n');
23+
Summary = lines.FirstOrDefault();
24+
25+
FullMessage = fullMessage;
26+
var detailsLines = lines
27+
.Skip(1)
28+
.SkipWhile(string.IsNullOrEmpty)
29+
.ToList();
30+
Details = detailsLines.Any(x => !string.IsNullOrWhiteSpace(x))
31+
? string.Join(Environment.NewLine, detailsLines).Trim()
32+
: null;
33+
}
34+
35+
public bool Equals(CommitMessage other)
36+
{
37+
if (ReferenceEquals(other, null))
38+
{
39+
return false;
40+
}
41+
42+
return string.Equals(Summary, other.Summary)
43+
&& string.Equals(Details, other.Details);
44+
}
45+
46+
public override bool Equals(object obj)
47+
{
48+
return Equals(obj as CommitMessage);
49+
}
50+
51+
public override int GetHashCode()
52+
{
53+
return Tuple.Create(Summary, Details).GetHashCode();
54+
}
55+
}
56+
}

‎src/GitHub.Extensions/StringExtensions.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Diagnostics.CodeAnalysis;
34
using System.Globalization;
45
using System.IO;
56
using System.Linq;
67
using System.Text;
8+
using System.Text.RegularExpressions;
79

810
namespace GitHub.Extensions
911
{
@@ -196,5 +198,29 @@ public static Uri ToUriSafe(this string url)
196198
Uri.TryCreate(url, UriKind.Absolute, out uri);
197199
return uri;
198200
}
201+
202+
/// <summary>
203+
/// Returns an alphanumeric sentence cased string with dashes and underscores as spaces.
204+
/// </summary>
205+
/// <param name="s">The string to format.</param>
206+
[SuppressMessage("Microsoft.Globalization", "CA1308:NormalizeStringsToUppercase")]
207+
public static string Humanize(this string s)
208+
{
209+
if (String.IsNullOrWhiteSpace(s))
210+
{
211+
return s;
212+
}
213+
214+
var matches = Regex.Matches(s, @"[a-zA-Z\d]{1,}", RegexOptions.None);
215+
216+
if (matches.Count == 0)
217+
{
218+
return s;
219+
}
220+
221+
var result = matches.Cast<Match>().Select(match => match.Value.ToLower(CultureInfo.InvariantCulture));
222+
var combined = String.Join(" ", result);
223+
return Char.ToUpper(combined[0], CultureInfo.InvariantCulture) + combined.Substring(1);
224+
}
199225
}
200226
}

‎test/UnitTests/GitHub.App/ViewModels/PullRequestCreationViewModelTests.cs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ public void TargetBranchDisplayNameIncludesRepoOwnerWhenFork()
132132
[InlineData("repo-name-8", "source-repo-owner", "master", true, false, "target-repo-owner", "master", "title", "description")]
133133
[InlineData("repo-name-9", "source-repo-owner", "source-branch", false, false, "source-repo-owner", "target-branch", "title", null)]
134134
[InlineData("repo-name-10", "source-repo-owner", "source-branch", false, false, "source-repo-owner", "master", "title", "description")]
135+
[InlineData("repo-name-11", "source-repo-owner", "source-branch", false, false, "source-repo-owner", "master", null, null)]
135136
public async Task CreatingPRs(
136137
string repoName, string sourceRepoOwner, string sourceBranchName,
137138
bool repoIsFork, bool sourceBranchIsTracking,
@@ -151,30 +152,31 @@ public async Task CreatingPRs(
151152
var ms = data.ModelService;
152153

153154
var prservice = new PullRequestService(data.GitClient, data.GitService, data.ServiceProvider.GetOperatingSystem(), Substitute.For<IUsageTracker>());
154-
var vm = new PullRequestCreationViewModel(data.RepositoryHost, data.ActiveRepo, prservice, data.NotificationService);
155+
prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Empty<string>());
155156

157+
var vm = new PullRequestCreationViewModel(data.RepositoryHost, data.ActiveRepo, prservice, data.NotificationService);
156158
vm.Initialize();
157159

158-
// the user has to input this
159-
vm.PRTitle = title;
160-
161-
// this is optional
162-
if (body != null)
163-
vm.Description = body;
164-
165160
// the TargetBranch property gets set to whatever the repo default is (we assume master here),
166161
// so we only set it manually to emulate the user selecting a different target branch
167162
if (targetBranchName != "master")
168163
vm.TargetBranch = new BranchModel(targetBranchName, targetRepo);
169164

165+
if (title != null)
166+
vm.PRTitle = title;
167+
168+
// this is optional
169+
if (body != null)
170+
vm.Description = body;
171+
170172
await vm.CreatePullRequest.ExecuteAsync();
171173

172174
var unused2 = gitClient.Received().Push(l2repo, sourceBranchName, remote);
173175
if (!sourceBranchIsTracking)
174176
unused2 = gitClient.Received().SetTrackingBranch(l2repo, sourceBranchName, remote);
175177
else
176178
unused2 = gitClient.DidNotReceiveWithAnyArgs().SetTrackingBranch(Args.LibGit2Repo, Args.String, Args.String);
177-
var unused = ms.Received().CreatePullRequest(activeRepo, targetRepo, sourceBranch, targetBranch, title, body ?? String.Empty);
179+
var unused = ms.Received().CreatePullRequest(activeRepo, targetRepo, sourceBranch, targetBranch, title ?? "Source branch", body ?? String.Empty);
178180
}
179181

180182
[Fact]

0 commit comments

Comments
 (0)