Merge changes I8e8c39c1,I98af48fc,Iafae1325,I2b5faa63,I7bc5d749, ... * changes: Add an FAQ about where to store the global plugin configuration Document that code owner approvals always apply to all files in the change Document features of the code-owners plugin on the about page Document the non-compatibility with the owners plugin Document the functionality of the owners plugin Document the compatibility with the find-owners plugin Document the functionality of the find-owners plugin Add latency metrics for relevant operations Allow to change more project config parameters via REST
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java index 68573a4..1596e23 100644 --- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java +++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java
@@ -83,8 +83,7 @@ if (oldApprovals.get(requiredApproval.labelType().getName()) == null) { // If oldApprovals doesn't contain the label or if the labels value in it is null, the label // was not changed. - // This either means that the user only voted on unrelated labels, or that the user applied - // the exact same code owner approval again, that was already present. + // This means that the user only voted on unrelated labels. return Optional.empty(); } @@ -197,6 +196,7 @@ oldApprovals, approvals, requiredApproval); + return Optional.empty(); } if (ownedPaths.size() <= maxPathsInChangeMessage) {
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java index 761472b..9bae548 100644 --- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java +++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
@@ -34,6 +34,7 @@ import com.google.inject.Inject; import java.util.Collection; import java.util.HashMap; +import java.util.regex.Pattern; import org.junit.Test; /** @@ -90,7 +91,7 @@ } @Test - public void changeMessageNotExtended_sameCodeOwnerApprovalAppliedAgain() throws Exception { + public void changeMessageExtended_sameCodeOwnerApprovalAppliedAgain() throws Exception { codeOwnerConfigOperations .newCodeOwnerConfig() .project(project) @@ -109,13 +110,22 @@ // Apply the Code-Review+1 approval again recommend(changeId); - // Check that no new change message was added. - // Gerrit core omits the change message if no vote was changed. - assertThat(gApi.changes().id(changeId).get().messages.size()).isEqualTo(messageCount); + Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; + // Check that a new change message was added. + assertThat(messages.size()).isEqualTo(messageCount + 1); + + assertThat(Iterables.getLast(messages).message) + .isEqualTo( + String.format( + "Patch Set 1: Code-Review+1\n\n" + + "By voting Code-Review+1 the following files are still code-owner approved by" + + " %s:\n" + + "* %s\n", + admin.fullName(), path)); } @Test - public void changeMessageNotExtended_sameCodeOwnerApprovalAppliedAgainTogetherWithOtherLabel() + public void changeMessageExtended_sameCodeOwnerApprovalAppliedAgainTogetherWithOtherLabel() throws Exception { LabelDefinitionInput input = new LabelDefinitionInput(); input.values = ImmutableMap.of("+1", "Other", " 0", "Approved"); @@ -150,14 +160,25 @@ reviewInput.labels.put("Other", (short) 1); gApi.changes().id(changeId).current().review(reviewInput); - // The message is unchanged, since reapplying the same code owner approval is ignored by Gerrit - // core (the change message only mentions the new vote, but not the reapplied vote). Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; - assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Other+1"); + assertThat(Iterables.getLast(messages).message) + .matches( + Pattern.quote("Patch Set 1: ") + + "(" + + Pattern.quote("Code-Review+1 Other+1") + + "|" + + Pattern.quote("Other+1 Code-Review+1") + + ")" + + Pattern.quote( + String.format( + "\n\nBy voting Code-Review+1 the following files are still code-owner approved by" + + " %s:\n" + + "* %s\n", + admin.fullName(), path))); } @Test - public void changeMessageNotExtended_sameCodeOwnerApprovalAppliedAgainTogetherWithComment() + public void changeMessageExtended_sameCodeOwnerApprovalAppliedAgainTogetherWithComment() throws Exception { LabelDefinitionInput input = new LabelDefinitionInput(); input.values = ImmutableMap.of("+1", "Other", " 0", "Approved"); @@ -193,14 +214,19 @@ commentInput.message = "some comment"; commentInput.path = path; ReviewInput reviewInput = ReviewInput.recommend(); - reviewInput.comments = reviewInput.comments = new HashMap<>(); + reviewInput.comments = new HashMap<>(); reviewInput.comments.put(commentInput.path, Lists.newArrayList(commentInput)); gApi.changes().id(changeId).current().review(reviewInput); - // The message is unchanged, since reapplying the same code owner approval is ignored by Gerrit - // core (the change message only mentions the comment, but not the reapplied vote). Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; - assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1:\n\n" + "(1 comment)"); + assertThat(Iterables.getLast(messages).message) + .isEqualTo( + String.format( + "Patch Set 1: Code-Review+1\n\n(1 comment)\n\n" + + "By voting Code-Review+1 the following files are still code-owner approved by" + + " %s:\n" + + "* %s\n", + admin.fullName(), path)); } @Test @@ -864,7 +890,7 @@ commentInput.message = "some comment"; commentInput.path = path; ReviewInput reviewInput = ReviewInput.recommend(); - reviewInput.comments = reviewInput.comments = new HashMap<>(); + reviewInput.comments = new HashMap<>(); reviewInput.comments.put(commentInput.path, Lists.newArrayList(commentInput)); gApi.changes().id(changeId).current().review(reviewInput);
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md index 5c21f29..aa9c064 100644 --- a/resources/Documentation/config.md +++ b/resources/Documentation/config.md
@@ -46,7 +46,7 @@ This allows branches to opt-out of the code owners functionality.\ Can be set multiple times.\ Can be overridden per project by setting - [codeOwners.disabledBrancg](#codeOwnersDisabledBranch) in + [codeOwners.disabledBranch](#codeOwnersDisabledBranch) in `@PLUGIN@.config`.\ By default unset.