commit 912c1b41ff7322f0f37c790172ada455d12e92ae
author Emmanuel Debanne <emmanuel.debanne@gmail.com> Tue Sep 05 15:22:24 2017 +0200
committer Emmanuel Debanne <emmanuel.debanne@gmail.com> Tue Sep 05 17:26:53 2017 +0200
tree e27df8179f9cbc6395c83bfe6ba27e20ba2ecf6e
parent cea7a4ec2926519372a62c482c8e7b4210f8d3fa

Improve logs about submittability

Change-Id: I3d169e96c0095bd6e40f6bff2902ebb0a533ac6b

src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java

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 fcd4b9b..8a3147f 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
@@ -181,12 +181,9 @@
       throws OrmException, RestApiException, NoSuchChangeException, IOException, UpdateException {
     if (atomicityHelper.isSubmittable(change.project, change.number)) {
       if (atomicityHelper.isAtomicReview(change)) {
-        log.info(
-            String.format(
-                "Change %d is submittable. Will try to merge all related changes.", change.number));
         attemptToMergeAtomic(change);
       } else {
-        log.info(String.format("Change %d is submittable. Submitting ...", change.number));
+        log.info("Submitting non-atomic change {}...", change.number);
         atomicityHelper.mergeReview(change.project, change.number);
       }
     }
@@ -224,29 +221,29 @@
             .query("status: open AND topic: " + change.topic)
             .withOption(ListChangesOption.CURRENT_REVISION)
             .get());
-    boolean submittable = true;
-    boolean mergeable = true;
     for (final ChangeInfo info : related) {
-      if (!info.mergeable) {
-        mergeable = false;
-      }
-      boolean passesPrologRules = atomicityHelper.isSubmittable(info.project, info._number);
-      boolean dependsOnNonMergedCommit = atomicityHelper.hasDependentReview(info.project, info._number);
-      if (!passesPrologRules || dependsOnNonMergedCommit) {
-        submittable = false;
+      if (!atomicityHelper.isSubmittable(info.project, info._number)) {
+        log.info("Change {} is not submittable because same topic change {} has not all approvals.", change.number, info._number);
+        return;
       }
     }
 
-    if (submittable) {
-      if (mergeable) {
-        log.debug(String.format("Change %d is mergeable", change.number));
-        for (final ChangeInfo info : related) {
-          atomicityHelper.mergeReview(info.project, info._number);
+    for (final ChangeInfo info : related) {
+      boolean dependsOnNonMergedCommit = atomicityHelper.hasDependentReview(info.project, info._number);
+      if (!info.mergeable || dependsOnNonMergedCommit) {
+        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);
         }
-      } else {
-        reviewUpdater.commentOnReview(change.project, change.number, AutomergeConfig.CANT_MERGE_COMMENT_FILE);
+        return;
       }
     }
+
+    log.info("Submitting atomic change {}...", change.number);
+    for (final ChangeInfo info : related) {
+      atomicityHelper.mergeReview(info.project, info._number);
+    }
   }
 
   private void processNewAtomicPatchSet(Change change) {