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 <sop@google.com>
This commit is contained in:
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -201,6 +201,7 @@ class SystemConfigProvider implements Provider<SystemConfig> {
|
||||
cat = new ApprovalCategory(new ApprovalCategory.Id("CRVW"), "Code Review");
|
||||
cat.setPosition((short) 1);
|
||||
cat.setAbbreviatedName("R");
|
||||
cat.setCopyMinScore(true);
|
||||
vals = new ArrayList<ApprovalCategoryValue>();
|
||||
vals.add(value(cat, 2, "Looks good to me, approved"));
|
||||
vals.add(value(cat, 1, "Looks good to me, but someone else must approve"));
|
||||
|
||||
@@ -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.
|
||||
//
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user