Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

376: Link to commits when number of commits exceeds 10 #603

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -160,13 +160,13 @@ static ArchiveItem from(PullRequest pr, Repository localRepo, HostUserToEmailAut
() -> "",
() -> {
if (lastBase.equals(base)) {
return ArchiveMessages.composeIncrementalRevision(localRepo, hostUserToCommitterName(hostUserToEmailAuthor, pr.author()), head, lastHead);
return ArchiveMessages.composeIncrementalRevision(pr, localRepo, hostUserToCommitterName(hostUserToEmailAuthor, pr.author()), head, lastHead);
} else {
var rebasedLastHead = rebasedLastHead(localRepo, base, lastHead);
if (rebasedLastHead.isPresent()) {
return ArchiveMessages.composeRebasedIncrementalRevision(localRepo, hostUserToCommitterName(hostUserToEmailAuthor, pr.author()), head, rebasedLastHead.get());
return ArchiveMessages.composeRebasedIncrementalRevision(pr, localRepo, hostUserToCommitterName(hostUserToEmailAuthor, pr.author()), head, rebasedLastHead.get());
} else {
return ArchiveMessages.composeFullRevision(localRepo, hostUserToCommitterName(hostUserToEmailAuthor, pr.author()), base, head);
return ArchiveMessages.composeFullRevision(pr, localRepo, hostUserToCommitterName(hostUserToEmailAuthor, pr.author()), base, head);
}
}
},
@@ -77,6 +77,10 @@ private static List<CommitMetadata> commits(Repository localRepo, Hash first, Ha
}
}

private static URI commitsLink(PullRequest pr, Hash first, Hash last) {
return pr.repository().webUrl(first.abbreviate(), last.abbreviate());
}

private static String formatNumber(int number) {
switch (number) {
case 0: return "no";
@@ -98,32 +102,35 @@ private static String describeCommits(List<CommitMetadata> commits, String adjec
" commit" + (commits.size() != 1 ? "s" : "");
}

private static Optional<String> formatCommitMessagesFull(List<CommitMetadata> commits) {
private static Optional<String> formatCommitMessagesFull(List<CommitMetadata> commits, URI commitsLink) {
if (commits.size() == 0) {
return Optional.empty();
} else if (commits.size() == 1) {
return Optional.of(formatSingleCommit(commits.get(0)));
} else {
return Optional.of(commits.stream()
var commitSummary = commits.stream()
.limit(10)
.map(ArchiveMessages::formatCommitInList)
.collect(Collectors.joining("\n")));
.collect(Collectors.joining("\n"));
if (commits.size() > 10) {
commitSummary += "\n - ... and " + (commits.size() - 10) + " more: ";
commitSummary += commitsLink.toString();
}
return Optional.of(commitSummary);
}
}

private static Optional<String> formatCommitMessagesBrief(List<CommitMetadata> commits) {
return formatCommitMessagesBrief(commits, 100);
}

private static Optional<String> formatCommitMessagesBrief(List<CommitMetadata> commits, int maxEntries) {
private static Optional<String> formatCommitMessagesBrief(List<CommitMetadata> commits, URI commitsLink) {
if (commits.size() == 0) {
return Optional.empty();
} else {
var commitSummary = commits.stream()
.limit(maxEntries)
.limit(10)
.map(ArchiveMessages::formatCommitBrief)
.collect(Collectors.joining("\n"));
if (commits.size() > maxEntries) {
commitSummary += "\n - ...omitting " + (commits.size() - maxEntries) + " further commits.";
if (commits.size() > 10) {
commitSummary += "\n - ... and " + (commits.size() - 10) + " more: ";
commitSummary += commitsLink.toString();
}
return Optional.of(commitSummary);
}
@@ -169,7 +176,7 @@ static String composeConversation(PullRequest pr) {
return filteredBody;
}

static String composeIncrementalRevision(Repository localRepository, String author, Hash head, Hash lastHead) {
static String composeIncrementalRevision(PullRequest pr, Repository localRepository, String author, Hash head, Hash lastHead) {
var ret = new StringBuilder();

var incrementalUpdate = false;
@@ -178,7 +185,8 @@ static String composeIncrementalRevision(Repository localRepository, String auth
} catch (IOException ignored) {
}
var commits = commits(localRepository, lastHead, head);
var newCommitMessages = formatCommitMessagesFull(commits);
var commitsLink = commitsLink(pr, lastHead, head);
var newCommitMessages = formatCommitMessagesFull(commits, commitsLink);
if (incrementalUpdate) {
ret.append(author);
ret.append(" has updated the pull request incrementally");
@@ -201,15 +209,16 @@ static String composeIncrementalRevision(Repository localRepository, String auth
return ret.toString();
}

static String composeRebasedIncrementalRevision(Repository localRepository, String author, Hash head, Hash lastHead) {
static String composeRebasedIncrementalRevision(PullRequest pr, Repository localRepository, String author, Hash head, Hash lastHead) {
var ret = new StringBuilder();

ret.append(author);
ret.append(" has updated the pull request with a new target base due to a merge or a rebase. ");
ret.append("The incremental webrev excludes the unrelated changes brought in by the merge/rebase.");

var commits = commits(localRepository, lastHead, head);
var newCommitMessages = formatCommitMessagesFull(commits);
var commitsLink = commitsLink(pr, lastHead, head);
var newCommitMessages = formatCommitMessagesFull(commits, commitsLink);
var commitsDescription = describeCommits(commits, "additional");
newCommitMessages.ifPresent(m -> ret.append(" The pull request contains ")
.append(commitsDescription)
@@ -218,14 +227,15 @@ static String composeRebasedIncrementalRevision(Repository localRepository, Stri
return ret.toString();
}

static String composeFullRevision(Repository localRepository, String author, Hash base, Hash head) {
static String composeFullRevision(PullRequest pr, Repository localRepository, String author, Hash base, Hash head) {
var ret = new StringBuilder();

ret.append(author);
ret.append(" has updated the pull request with a new target base due to a merge or a rebase.");

var commits = commits(localRepository, base, head);
var newCommitMessages = formatCommitMessagesFull(commits);
var commitsLink = commitsLink(pr, base, head);
var newCommitMessages = formatCommitMessagesFull(commits, commitsLink);
var commitsDescription = describeCommits(commits, "");
newCommitMessages.ifPresent(m -> ret.append(" The pull request now contains ")
.append(commitsDescription)
@@ -249,9 +259,10 @@ static String composeReplyFooter(PullRequest pr) {
// When changing this, ensure that the PR pattern in the notifier still matches
static String composeConversationFooter(PullRequest pr, URI issueProject, String projectPrefix, Repository localRepo, WebrevDescription webrev, Hash base, Hash head) {
var commits = commits(localRepo, base, head);
var commitsLink = commitsLink(pr, base, head);
var issueString = issueUrl(pr, issueProject, projectPrefix).map(url -> " Issue: " + url + "\n").orElse("");
return "Commit messages:\n" +
formatCommitMessagesBrief(commits).orElse("") + "\n\n" +
formatCommitMessagesBrief(commits, commitsLink).orElse("") + "\n\n" +
"Changes: " + pr.changeUrl() + "\n" +
" Webrev: " + webrev.uri().toString() + "\n" +
issueString +
@@ -263,6 +274,7 @@ static String composeConversationFooter(PullRequest pr, URI issueProject, String

static String composeMergeConversationFooter(PullRequest pr, Repository localRepo, List<WebrevDescription> webrevs, Hash base, Hash head) {
var commits = commits(localRepo, base, head);
var commitsLink = commitsLink(pr, base, head);
String webrevLinks;
if (webrevs.size() > 0) {
var containsConflicts = webrevs.stream().anyMatch(w -> w.type().equals(WebrevDescription.Type.MERGE_CONFLICT));
@@ -281,7 +293,7 @@ static String composeMergeConversationFooter(PullRequest pr, Repository localRep
webrevLinks = "The merge commit only contains trivial merges, so no merge-specific webrevs have been generated.\n\n";
}
return "Commit messages:\n" +
formatCommitMessagesBrief(commits, 10).orElse("") + "\n\n" +
formatCommitMessagesBrief(commits, commitsLink).orElse("") + "\n\n" +
webrevLinks +
"Changes: " + pr.changeUrl() + "\n" +
" Stats: " + stats(localRepo, base, head) + "\n" +