diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt index b20cd74131..3c0f0ebdc4 100644 --- a/Documentation/config-labels.txt +++ b/Documentation/config-labels.txt @@ -222,7 +222,8 @@ determining whether a change is submittable. === `label.Label-Name.copyMinScore` If true, the lowest possible negative value for the label is copied -forward when a new patch set is uploaded. +forward when a new patch set is uploaded. Defaults to false, except +for All-Projects which has it true by default. [[label_copyMaxScore]] === `label.Label-Name.copyMaxScore` @@ -230,7 +231,7 @@ forward when a new patch set is uploaded. If true, the highest possible positive value for the label is copied forward when a new patch set is uploaded. This can be used to enable sticky approvals, reducing turn-around for trivial cleanups prior to -submitting a change. +submitting a change. Defaults to false. [[label_copyAllScoresOnTrivialRebase]] === `label.Label-Name.copyAllScoresOnTrivialRebase` diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java index e9e3ffbc36..efb461511b 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java @@ -48,16 +48,16 @@ public class LabelTypeIT extends AbstractDaemonTest { public void setUp() throws Exception { ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); codeReview = checkNotNull(cfg.getLabelSections().get("Code-Review")); - codeReview.setCopyMinScore(false); - codeReview.setCopyMaxScore(false); - codeReview.setCopyAllScoresOnTrivialRebase(false); - codeReview.setCopyAllScoresIfNoCodeChange(false); codeReview.setDefaultValue((short)-1); saveProjectConfig(cfg); } @Test public void noCopyMinScoreOnRework() throws Exception { + //allProjects only has it true by default + codeReview.setCopyMinScore(false); + saveLabelConfig(); + PushOneCommit.Result r = createChange(); revision(r).review(ReviewInput.reject()); assertApproval(r, -2); @@ -71,7 +71,7 @@ public class LabelTypeIT extends AbstractDaemonTest { saveLabelConfig(); PushOneCommit.Result r = createChange(); revision(r).review(ReviewInput.reject()); - //assertApproval(r, -2); + assertApproval(r, -2); r = amendChange(r.getChangeId()); assertApproval(r, -2); } @@ -122,6 +122,22 @@ public class LabelTypeIT extends AbstractDaemonTest { assertApproval(r, 0); } + @Test + public void noCopyAllScoresIfNoChange() throws Exception { + codeReview.setCopyAllScoresIfNoChange(false); + saveLabelConfig(); + PushOneCommit.Result patchSet = readyPatchSetForNoChangeRebase(); + rebase(patchSet); + assertApproval(patchSet, 0); + } + + @Test + public void copyAllScoresIfNoChange() throws Exception { + PushOneCommit.Result patchSet = readyPatchSetForNoChangeRebase(); + rebase(patchSet); + assertApproval(patchSet, 1); + } + @Test public void noCopyAllScoresIfNoCodeChange() throws Exception { String file = "a.txt"; @@ -267,6 +283,34 @@ public class LabelTypeIT extends AbstractDaemonTest { .get()); } + private PushOneCommit.Result readyPatchSetForNoChangeRebase() + throws Exception { + String file = "a.txt"; + String contents = "contents"; + + PushOneCommit push = pushFactory.create(db, admin.getIdent(), + PushOneCommit.SUBJECT, file, contents); + PushOneCommit.Result base = push.to(git, "refs/for/master"); + merge(base); + + push = pushFactory.create(db, admin.getIdent(), + PushOneCommit.SUBJECT, file, contents + "M"); + PushOneCommit.Result basePlusM = push.to(git, "refs/for/master"); + merge(basePlusM); + + push = pushFactory.create(db, admin.getIdent(), + PushOneCommit.SUBJECT, file, contents); + PushOneCommit.Result basePlusMMinusM = push.to(git, "refs/for/master"); + merge(basePlusMMinusM); + + git.checkout().setName(base.getCommit().name()).call(); + push = pushFactory.create(db, admin.getIdent(), + PushOneCommit.SUBJECT, file, contents + "MM"); + PushOneCommit.Result patchSet = push.to(git, "refs/for/master"); + revision(patchSet).review(ReviewInput.recommend()); + return patchSet; + } + private void saveLabelConfig() throws Exception { ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); cfg.getLabelSections().clear(); diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java index 315be845bd..cf6f7564fb 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java @@ -25,6 +25,13 @@ import java.util.List; import java.util.Map; public class LabelType { + public static final boolean DEF_CAN_OVERRIDE = true; + public static final boolean DEF_COPY_ALL_SCORES_IF_NO_CHANGE = true; + public static final boolean DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE = false; + public static final boolean DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = false; + public static final boolean DEF_COPY_MAX_SCORE = false; + public static final boolean DEF_COPY_MIN_SCORE = false; + public static LabelType withDefaultValues(String name) { checkName(name); List values = new ArrayList<>(2); @@ -127,6 +134,12 @@ public class LabelType { maxPositive = values.get(values.size() - 1).getValue(); } } + setCanOverride(DEF_CAN_OVERRIDE); + setCopyAllScoresIfNoChange(DEF_COPY_ALL_SCORES_IF_NO_CHANGE); + setCopyAllScoresIfNoCodeChange(DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE); + setCopyAllScoresOnTrivialRebase(DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE); + setCopyMaxScore(DEF_COPY_MAX_SCORE); + setCopyMinScore(DEF_COPY_MIN_SCORE); } public String getName() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java index c749a2d9db..020c94ae71 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java @@ -691,17 +691,23 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. KEY_DEFAULT_VALUE, dv, name))); } label.setCopyMinScore( - rc.getBoolean(LABEL, name, KEY_COPY_MIN_SCORE, false)); + rc.getBoolean(LABEL, name, KEY_COPY_MIN_SCORE, + LabelType.DEF_COPY_MIN_SCORE)); label.setCopyMaxScore( - rc.getBoolean(LABEL, name, KEY_COPY_MAX_SCORE, false)); + rc.getBoolean(LABEL, name, KEY_COPY_MAX_SCORE, + LabelType.DEF_COPY_MAX_SCORE)); label.setCopyAllScoresOnTrivialRebase( - rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE, false)); + rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE, + LabelType.DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE)); label.setCopyAllScoresIfNoCodeChange( - rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE, false)); + rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE, + LabelType.DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE)); label.setCopyAllScoresIfNoChange( - rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE, true)); + rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE, + LabelType.DEF_COPY_ALL_SCORES_IF_NO_CHANGE)); label.setCanOverride( - rc.getBoolean(LABEL, name, KEY_CAN_OVERRIDE, true)); + rc.getBoolean(LABEL, name, KEY_CAN_OVERRIDE, + LabelType.DEF_CAN_OVERRIDE)); label.setRefPatterns(getStringListOrNull(rc, LABEL, name, KEY_Branch)); labelSections.put(name, label); } @@ -1034,37 +1040,22 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. toUnset.remove(name); rc.setString(LABEL, name, KEY_FUNCTION, label.getFunctionName()); rc.setInt(LABEL, name, KEY_DEFAULT_VALUE, label.getDefaultValue()); - if (label.isCopyMinScore()) { - rc.setBoolean(LABEL, name, KEY_COPY_MIN_SCORE, true); - } else { - rc.unset(LABEL, name, KEY_COPY_MIN_SCORE); - } - if (label.isCopyMaxScore()) { - rc.setBoolean(LABEL, name, KEY_COPY_MAX_SCORE, true); - } else { - rc.unset(LABEL, name, KEY_COPY_MAX_SCORE); - } - if (label.isCopyAllScoresOnTrivialRebase()) { - rc.setBoolean(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE, true); - } else { - rc.unset(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE); - } - if (label.isCopyAllScoresIfNoCodeChange()) { - rc.setBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE, true); - } else { - rc.unset(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE); - } - if (!label.isCopyAllScoresIfNoChange()) { - rc.setBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE, false); - } else { - rc.unset(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE); - } - if (!label.canOverride()) { - rc.setBoolean(LABEL, name, KEY_CAN_OVERRIDE, false); - } else { - rc.unset(LABEL, name, KEY_CAN_OVERRIDE); - } + setBooleanConfigKey(rc, name, KEY_COPY_MIN_SCORE, label.isCopyMinScore(), + LabelType.DEF_COPY_MIN_SCORE); + setBooleanConfigKey(rc, name, KEY_COPY_MAX_SCORE, label.isCopyMaxScore(), + LabelType.DEF_COPY_MAX_SCORE); + setBooleanConfigKey(rc, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE, + label.isCopyAllScoresOnTrivialRebase(), + LabelType.DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE); + setBooleanConfigKey(rc, name, KEY_COPY_ALL_SCORES_IF_NO_CODE_CHANGE, + label.isCopyAllScoresIfNoCodeChange(), + LabelType.DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE); + setBooleanConfigKey(rc, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE, + label.isCopyAllScoresIfNoChange(), + LabelType.DEF_COPY_ALL_SCORES_IF_NO_CHANGE); + setBooleanConfigKey(rc, name, KEY_CAN_OVERRIDE, label.canOverride(), + LabelType.DEF_CAN_OVERRIDE); List values = Lists.newArrayListWithCapacity(label.getValues().size()); for (LabelValue value : label.getValues()) { @@ -1078,6 +1069,15 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. } } + private static void setBooleanConfigKey( + Config rc, String name, String key, boolean value, boolean defaultValue) { + if (value == defaultValue) { + rc.unset(LABEL, name, key); + } else { + rc.setBoolean(LABEL, name, key, value); + } + } + private void savePluginSections(Config rc) { List existing = Lists.newArrayList(rc.getSubsections(PLUGIN)); for (String name : existing) {