From b747cae7030a3853fa3a369877832806e10fe741 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 7 Feb 2018 19:16:27 +0900 Subject: [PATCH 1/4] ReviewerSuggestion: More Javadoc improvements Change-Id: I8fd165867f2fdc85aab6630e6c460620ab818df8 --- .../gerrit/server/change/ReviewerSuggestion.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java index f64c9d04f0..198a5fde04 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java @@ -29,14 +29,15 @@ import java.util.Set; @ExtensionPoint public interface ReviewerSuggestion { /** - * Reviewer suggestion. + * Suggest reviewers to add to a change. * * @param project The name key of the project the suggestion is for. - * @param changeId The changeId that the suggestion is for. Can be an {@code null}. - * @param query The query as typed by the user. Can be an {@code null}. + * @param changeId The changeId that the suggestion is for. Can be {@code null}. + * @param query The query as typed by the user. Can be {@code null}. * @param candidates A set of candidates for the ranking. Can be empty. - * @return Set of suggested reviewers as a tuple of account id and score. The account ids listed - * here don't have to be a part of candidates. + * @return Set of {@link SuggestedReviewer}s. The {@link + * com.google.gerrit.reviewdb.client.Account.Id}s listed here don't have to be included in + * {@code candidates}. */ Set suggestReviewers( Project.NameKey project, From d19d0d307ad19c092d18795796f88e660d9d1681 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 7 Feb 2018 21:27:03 +0900 Subject: [PATCH 2/4] ReviewerRecommender: Prevent NPE when changeNotes is null The changeNotes passed into suggestReviewers is possibly null, and there is a check for this later in the method before attempting to remove the change owner and existing reviewers from the list of recommendations. However the changeNotes was dereferenced to get the changeId to pass into the plugin provided suggestReviewers method. The suggestReviewers method annotates changeId as @Nullable, so we can simply pass null there when changeNotes is null. Change-Id: I4946b858dac05132cc65ebc6f4966a2fa0b23c11 --- .../main/java/com/google/gerrit/server/ReviewerRecommender.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java index 6a307d64e6..4575ef265d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java @@ -134,7 +134,7 @@ public class ReviewerRecommender { .get() .suggestReviewers( projectControl.getProject().getNameKey(), - changeNotes.getChangeId(), + changeNotes != null ? changeNotes.getChangeId() : null, query, reviewerScores.keySet())); String key = plugin.getPluginName() + "-" + plugin.getExportName(); From 350977680031edcdcdd6e51b9100b76333ba9101 Mon Sep 17 00:00:00 2001 From: Jacek Centkowski Date: Wed, 7 Feb 2018 12:44:05 +0100 Subject: [PATCH 3/4] Provide mvn command output when VERBOSE set According to [1] when CalledProcessError is thrown it contains failed command output. Print it out when command fails in VERBOSE mode so that it can be easier determined what went wrong. [1] https://docs.python.org/2/library/subprocess.html#subprocess.CalledProcessError Change-Id: I6b31bc7a4e93a947366e1381430a3b591fdab172 Signed-off-by: Jacek Centkowski --- tools/maven/mvn.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/maven/mvn.py b/tools/maven/mvn.py index f7b5aa82e9..a0939162a3 100755 --- a/tools/maven/mvn.py +++ b/tools/maven/mvn.py @@ -16,7 +16,7 @@ from __future__ import print_function from optparse import OptionParser from os import path, environ -from subprocess import check_output +from subprocess import check_output, CalledProcessError from sys import stderr opts = OptionParser() @@ -67,6 +67,8 @@ for spec in args.s: except Exception as e: print('%s command failed: %s\n%s' % (args.a, ' '.join(exe), e), file=stderr) + if environ.get('VERBOSE') and isinstance(e, CalledProcessError): + print('Command output\n%s' % e.output, file=stderr) exit(1) From 2ce2c9238282d5db87d722ebdf5247b23884a530 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Thu, 8 Feb 2018 07:00:14 +0100 Subject: [PATCH 4/4] Bazel: Silent zip output in gerrit-antlr:query_antlr rule @bazel_tools//tools/zip:zipper was replaced with zip command in Ib2ce3e2c19, but "-q" option was missed to be passed to zip command. Change-Id: I3d203b632f8d4b12b89a08bf15eddcbf68e835ab --- gerrit-antlr/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gerrit-antlr/BUILD b/gerrit-antlr/BUILD index 1176d17fa6..6c3910626e 100644 --- a/gerrit-antlr/BUILD +++ b/gerrit-antlr/BUILD @@ -13,7 +13,7 @@ genrule2( cmd = " && ".join([ "$(location //lib/antlr:antlr-tool) -o $$TMP $<", "cd $$TMP", - "zip $$ROOT/$@ $$(find . -type f )", + "zip -q $$ROOT/$@ $$(find . -type f )", ]), tools = [ "//lib/antlr:antlr-tool",