Provide default content for comments pushed by the plugin
Change-Id: I432a485387da50282e790101e13d017e78cb0c73
diff --git a/README.md b/README.md
index 9d5bee0..a96c3c5 100644
--- a/README.md
+++ b/README.md
@@ -12,3 +12,14 @@
when all approvals for all reviews are present, and all reviews are mergeable.
Requires Gerrit 2.14 or later.
+
+Configuration
+-------------
+
+```
+[automerge]
+ botEmail=admin@example.com
+[commentlink "change"]
+ match = "#/c/(\\d+)"
+ html = "<a href=\"/#/c/$1/\">$1</a>"
+```
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
index 15d3bce..dbf0776 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
@@ -227,7 +227,8 @@
log.info("Change {} is not mergeable because same topic change {} {}", change.number, info._number,
!info.mergeable ? "is non mergeable" : "depends on a non merged commit.");
if (!info.mergeable) {
- reviewUpdater.commentOnReview(change.project, change.number, AutomergeConfig.CANT_MERGE_COMMENT_FILE);
+ reviewUpdater.commentOnReview(change.project, change.number,
+ String.format(config.cantMergeGitConflict.getContent(), info._number));
}
return;
}
@@ -245,10 +246,10 @@
if (atomicityHelper.hasDependentReview(change.project, change.number)) {
log.info(String.format("Warn the user by setting -1 on change %d, as other atomic changes exists on the same repository.",
change.number));
- reviewUpdater.setMinusOne(change.project, change.number, AutomergeConfig.ATOMIC_REVIEWS_SAME_REPO_FILE);
+ reviewUpdater.setMinusOne(change.project, change.number, config.atomicReviewsSameRepo.getContent());
} else {
log.info(String.format("Detected atomic review on change %d.", change.number));
- reviewUpdater.commentOnReview(change.project, change.number, AutomergeConfig.ATOMIC_REVIEW_DETECTED_FILE);
+ reviewUpdater.commentOnReview(change.project, change.number, config.atomicReviewDetected.getContent());
}
} catch (RestApiException | IOException | OrmException | UpdateException e) {
throw new RuntimeException(e);
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
index 40d0811..55fa745 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
@@ -7,16 +7,17 @@
import org.eclipse.jgit.lib.Config;
import java.io.File;
-import java.io.IOException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class AutomergeConfig {
-
- public final static String ATOMIC_REVIEW_DETECTED_FILE = "atomic_review_detected.txt";
- public final static String ATOMIC_REVIEWS_SAME_REPO_FILE = "atomic_review_same_repo.txt";
public final static String AUTOMERGE_SECTION = "automerge";
public final static String BOT_EMAIL_KEY = "botEmail";
- public final static String CANT_MERGE_COMMENT_FILE = "cantmerge.txt";
+
+ public final PluginComment atomicReviewDetected;
+ public final PluginComment atomicReviewsSameRepo;
+ public final PluginComment cantMergeGitConflict;
private final static String defaultBotEmail = "qabot@criteo.com";
private final static String defaultTopicPrefix = "crossrepo/";
@@ -46,21 +47,22 @@
}
templatesPath = paths.etc_dir.toFile();
+
+ atomicReviewDetected = new PluginComment(getCommentPath("atomic_review_detected.txt"),
+ "This review is part of a cross-repository change.\n"
+ + "It will be submitted once all related reviews are submittable.");
+ atomicReviewsSameRepo = new PluginComment(getCommentPath("atomic_review_same_repo.txt"),
+ "This cross-repo review depends on a not merged commit that must be merged first.");
+ cantMergeGitConflict = new PluginComment(getCommentPath("cantmerge_git_conflict.txt"),
+ "This cross-repo review is blocked by a git conflict on change #/c/%d.");
}
public final String getBotEmail() {
return botEmail;
}
- /**
- * Return a comment from a file located in the gerrit etc_dir
- *
- * @param filename
- * @return a string containing a comment.
- * @throws IOException
- */
- public final String getTemplatesPath() {
- return templatesPath.getPath();
+ public final File getCommentPath(String fileName) {
+ return new File(templatesPath.getPath(), fileName);
}
public final String getTopicPrefix() {
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/PluginComment.java b/src/main/java/com/criteo/gerrit/plugins/automerge/PluginComment.java
new file mode 100644
index 0000000..a9c2cc0
--- /dev/null
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/PluginComment.java
@@ -0,0 +1,39 @@
+package com.criteo.gerrit.plugins.automerge;
+
+import com.google.common.base.Charsets;
+import com.google.common.io.Files;
+import java.io.File;
+import java.io.IOException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A comment pushed by the plugin to a Gerrit patchset.
+ */
+public class PluginComment {
+
+ private final static Logger log = LoggerFactory.getLogger(PluginComment.class);
+
+ private final File templatePath;
+ private final String defaultMessage;
+
+ PluginComment(File templatePath, String defaultMessage) {
+ this.templatePath = templatePath;
+ this.defaultMessage = defaultMessage;
+ }
+
+ /**
+ * Returns the comment message, possibly with interpolation placeholders.
+ * @return a string
+ */
+ String getContent() {
+ if (templatePath.exists()) {
+ try {
+ return Files.toString(templatePath, Charsets.UTF_8);
+ } catch (final IOException exc) {
+ log.error("Not able to read " + templatePath, exc);
+ }
+ }
+ return defaultMessage;
+ }
+}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
index af2e7a6..ee95240 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
@@ -1,12 +1,7 @@
package com.criteo.gerrit.plugins.automerge;
-import com.google.common.base.Charsets;
-import com.google.common.io.Files;
import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;
@@ -20,12 +15,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.File;
import java.io.IOException;
public class ReviewUpdater {
- private final static Logger log = LoggerFactory.getLogger(AutomaticMerger.class);
-
/**
* Prefix used in front of messages pushed to Gerrit by this plugin. This prefix is used to
* discriminate the messages emitted by the plugin from the other messages and avoid infinite
@@ -34,46 +26,26 @@
public static final String commentsPrefix = "[Autosubmitter] ";
@Inject
- ChangeData.Factory changeDataFactory;
-
- @Inject
- AutomergeConfig config;
-
- @Inject
- Provider<ReviewDb> db;
-
- @Inject
Provider<PostReview> reviewer;
@Inject
private AtomicityHelper atomicityHelper;
- public void commentOnReview(String project, int number, String commentTemplate) throws RestApiException, OrmException, IOException, NoSuchChangeException, UpdateException {
- ReviewInput comment = createComment(commentTemplate);
- applyComment(project, number, comment);
+ public void commentOnReview(String project, int number, String comment) throws RestApiException, OrmException, IOException, UpdateException {
+ ReviewInput reviewInput = createComment(comment);
+ applyComment(project, number, reviewInput);
}
- public void setMinusOne(String project, int number, String commentTemplate) throws RestApiException, OrmException, IOException, NoSuchChangeException, UpdateException {
- ReviewInput message = createComment(commentTemplate).label("Code-Review", -1);
+ public void setMinusOne(String project, int number, String comment) throws RestApiException, OrmException, IOException, UpdateException {
+ ReviewInput message = createComment(comment).label("Code-Review", -1);
applyComment(project, number, message);
}
- private ReviewInput createComment(final String commentTemplate) {
- return new ReviewInput().message(commentsPrefix + getCommentFromFile(commentTemplate));
+ private ReviewInput createComment(String comment) {
+ return new ReviewInput().message(commentsPrefix + comment);
}
- private String getCommentFromFile(final String filename) {
- try {
- return Files.toString(new File(config.getTemplatesPath(), filename), Charsets.UTF_8);
- } catch (final IOException exc) {
- final String errmsg =
- String.format("Cannot find %s file in gerrit etc dir. Please check your gerrit configuration", filename);
- log.error(errmsg);
- return errmsg;
- }
- }
-
- private void applyComment(String project, int number, ReviewInput comment) throws RestApiException, OrmException, IOException, NoSuchChangeException, UpdateException {
+ private void applyComment(String project, int number, ReviewInput comment) throws RestApiException, OrmException, IOException, UpdateException {
RevisionResource r = atomicityHelper.getRevisionResource(project, number);
reviewer.get().apply(r, comment);
}
diff --git a/test.rb b/test.rb
index f0d8cd0..1acfe59 100755
--- a/test.rb
+++ b/test.rb
@@ -49,7 +49,7 @@
commit0 = create_review(PROJECT1, "review0 on #{PROJECT1}")
commit0b = create_review(PROJECT1, "review0b on #{PROJECT1}", "crossrepo/topic1")
check_label(commit0b, "Code-Review", "-1")
- check_last_message_contains(commit0b, "atomic_review_same_repo.txt")
+ check_last_message_contains(commit0b, "This cross-repo review depends on a not merged commit")
end
def test_normal_topic_2_repos
