From fcba3c33e096dfcfeebf98f10c294d384d8c6f70 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 5 Sep 2009 16:07:27 -0700 Subject: [PATCH] Only copy blocking negative votes to replacement patch sets By default we only copy a blocking negative vote on "Code Review" category, leaving the "Verified" category alone. This actually makes a lot of sense given the standard meanings of these two categories. Since "Verified" tends to mean "compiles correctly" a negative score from a prior patch set does not mean that the current patch set also does not compile, it is actually more likely that the problem has been addressed in the new patch set. Change-Id: Ibedde352203c55d011a5d32c24f0ecc1352034df Signed-off-by: Shawn O. Pearce --- .../gerrit/client/reviewdb/ApprovalCategory.java | 12 ++++++++++++ .../gerrit/server/config/SystemConfigProvider.java | 1 + .../google/gerrit/server/ssh/commands/Receive.java | 5 ++++- src/main/webapp/WEB-INF/sql/upgrade017_018_mysql.sql | 5 +++++ .../webapp/WEB-INF/sql/upgrade017_018_postgres.sql | 6 ++++++ 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/gerrit/client/reviewdb/ApprovalCategory.java b/src/main/java/com/google/gerrit/client/reviewdb/ApprovalCategory.java index d4b644b873..2aa5df6714 100644 --- a/src/main/java/com/google/gerrit/client/reviewdb/ApprovalCategory.java +++ b/src/main/java/com/google/gerrit/client/reviewdb/ApprovalCategory.java @@ -106,6 +106,10 @@ public final class ApprovalCategory { @Column protected String functionName; + /** If set, the minimum score is copied during patch set replacement. */ + @Column + protected boolean copyMinScore; + protected ApprovalCategory() { } @@ -154,4 +158,12 @@ public final class ApprovalCategory { public void setFunctionName(final String name) { functionName = name; } + + public boolean isCopyMinScore() { + return copyMinScore; + } + + public void setCopyMinScore(final boolean copy) { + copyMinScore = copy; + } } diff --git a/src/main/java/com/google/gerrit/server/config/SystemConfigProvider.java b/src/main/java/com/google/gerrit/server/config/SystemConfigProvider.java index 66869ad98b..aabc23d47a 100644 --- a/src/main/java/com/google/gerrit/server/config/SystemConfigProvider.java +++ b/src/main/java/com/google/gerrit/server/config/SystemConfigProvider.java @@ -201,6 +201,7 @@ class SystemConfigProvider implements Provider { cat = new ApprovalCategory(new ApprovalCategory.Id("CRVW"), "Code Review"); cat.setPosition((short) 1); cat.setAbbreviatedName("R"); + cat.setCopyMinScore(true); vals = new ArrayList(); vals.add(value(cat, 2, "Looks good to me, approved")); vals.add(value(cat, 1, "Looks good to me, but someone else must approve")); diff --git a/src/main/java/com/google/gerrit/server/ssh/commands/Receive.java b/src/main/java/com/google/gerrit/server/ssh/commands/Receive.java index 4fa134a48d..8b79ee8c7f 100644 --- a/src/main/java/com/google/gerrit/server/ssh/commands/Receive.java +++ b/src/main/java/com/google/gerrit/server/ssh/commands/Receive.java @@ -1002,7 +1002,10 @@ final class Receive extends AbstractGitCommand { oldCC.add(a.getAccountId()); } - if (a.getValue() < 0 && a.getPatchSetId().equals(priorPatchSet)) { + final ApprovalType type = + approvalTypes.getApprovalType(a.getCategoryId()); + if (a.getPatchSetId().equals(priorPatchSet) + && type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) { // If there was a negative vote on the prior patch set, carry it // into this patch set. // diff --git a/src/main/webapp/WEB-INF/sql/upgrade017_018_mysql.sql b/src/main/webapp/WEB-INF/sql/upgrade017_018_mysql.sql index 2dabc61ba8..32017358b3 100644 --- a/src/main/webapp/WEB-INF/sql/upgrade017_018_mysql.sql +++ b/src/main/webapp/WEB-INF/sql/upgrade017_018_mysql.sql @@ -5,6 +5,11 @@ ALTER TABLE approval_categories ADD abbreviated_name VARCHAR(4); UPDATE approval_categories SET abbreviated_name = 'V' WHERE category_id = 'VRIF'; UPDATE approval_categories SET abbreviated_name = 'R' WHERE category_id = 'CRVW'; +ALTER TABLE approval_categories ADD copy_min_score CHAR(1); +UPDATE approval_categories SET copy_min_score = 'N'; +UPDATE approval_categories SET copy_min_score = 'Y' WHERE category_id = 'CRVW'; +ALTER TABLE approval_categories MODIFY copy_min_score CHAR(1) DEFAULT 'N' NOT NULL; + DROP TABLE patches; UPDATE schema_version SET version_nbr = 18; diff --git a/src/main/webapp/WEB-INF/sql/upgrade017_018_postgres.sql b/src/main/webapp/WEB-INF/sql/upgrade017_018_postgres.sql index 0e938796a2..87feb38072 100644 --- a/src/main/webapp/WEB-INF/sql/upgrade017_018_postgres.sql +++ b/src/main/webapp/WEB-INF/sql/upgrade017_018_postgres.sql @@ -9,6 +9,12 @@ ALTER TABLE approval_categories ADD abbreviated_name VARCHAR(4); UPDATE approval_categories SET abbreviated_name = 'V' WHERE category_id = 'VRIF'; UPDATE approval_categories SET abbreviated_name = 'R' WHERE category_id = 'CRVW'; +ALTER TABLE approval_categories ADD copy_min_score CHAR(1); +UPDATE approval_categories SET copy_min_score = 'N'; +UPDATE approval_categories SET copy_min_score = 'Y' WHERE category_id = 'CRVW'; +ALTER TABLE approval_categories ALTER COLUMN copy_min_score SET DEFAULT 'N'; +ALTER TABLE approval_categories ALTER COLUMN copy_min_score SET NOT NULL; + DROP TABLE patches; UPDATE schema_version SET version_nbr = 18;