Skip to content
Permalink
Browse files
234: Allow more granular jcheck reviewer configuration
Reviewed-by: rwestberg
  • Loading branch information
edvbld committed Jan 20, 2020
1 parent 3d62e28 commit daac177f6ee8181c898dab9dc09c593a621129eb
@@ -38,4 +38,4 @@ domain=openjdk.org
files=.*\.java$|.*\.yml$|.*\.gradle$|.*.\txt$

[checks "reviewers"]
minimum=1
reviewers=1
@@ -118,8 +118,7 @@ private static INI convert(INI old) {
config.add("message=Merge");

config.add("[checks \"reviewers\"]");
config.add("minimum=1");
config.add("role=contributor");
config.add("contributor=1");
config.add("ignore=duke");

config.add("[checks \"committer\"]");
@@ -29,6 +29,9 @@

import java.io.IOException;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.HashMap;
import java.util.stream.Collectors;
import java.util.logging.Logger;

@@ -42,20 +45,24 @@ public class ReviewersCheck extends CommitCheck {
this.utils = utils;
}

private boolean hasRole(Project project, String role, String username, int version) {
private String nextRequiredRole(String role, Map<String, Integer> count) {
switch (role) {
case "lead":
return project.isLead(username, version);
return count.get("reviewer") != 0 ?
"reviewer" : nextRequiredRole("reviewer", count);
case "reviewer":
return project.isReviewer(username, version);
return count.get("committer") != 0 ?
"committer" : nextRequiredRole("committer", count);
case "committer":
return project.isCommitter(username, version);
return count.get("author") != 0 ?
"author" : nextRequiredRole("author", count);
case "author":
return project.isAuthor(username, version);
return count.get("contributor") != 0 ?
"contributor" : nextRequiredRole("contributor", count);
case "contributor":
return census.isContributor(username);
return null;
default:
throw new IllegalStateException("Unsupported role: " + role);
throw new IllegalArgumentException("Unexpected role: " + role);
}
}

@@ -69,22 +76,19 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
var project = census.project(conf.general().project());
var version = conf.census().version();
var domain = conf.census().domain();
var role = conf.checks().reviewers().role();
var required = conf.checks().reviewers().minimum();

var numLeadRole = conf.checks().reviewers().lead();
var numReviewerRole = conf.checks().reviewers().reviewers();
var numCommitterRole = conf.checks().reviewers().committers();
var numAuthorRole = conf.checks().reviewers().authors();
var numContributorRole = conf.checks().reviewers().contributors();

var ignore = conf.checks().reviewers().ignore();
var reviewers = message.reviewers()
.stream()
.filter(r -> !ignore.contains(r))
.collect(Collectors.toList());

var actual = reviewers.stream()
.filter(reviewer -> hasRole(project, role, reviewer, version))
.count();
if (actual < required) {
log.finer("issue: too few reviewers found");
return iterator(new TooFewReviewersIssue(Math.toIntExact(actual), required, metadata));
}

var invalid = reviewers.stream()
.filter(r -> !census.isContributor(r))
.collect(Collectors.toList());
@@ -93,6 +97,55 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
return iterator(new InvalidReviewersIssue(invalid, project, metadata));
}

var requirements = Map.of(
"lead", numLeadRole,
"reviewer", numReviewerRole,
"committer", numCommitterRole,
"author", numAuthorRole,
"contributor", numContributorRole);

var roles = new HashMap<String, String>();
for (var reviewer : reviewers) {
String role = null;
if (project.isLead(reviewer, version)) {
role = "lead";
} else if (project.isReviewer(reviewer, version)) {
role = "reviewer";
} else if (project.isCommitter(reviewer, version)) {
role = "committer";
} else if (project.isAuthor(reviewer, version)) {
role = "author";
} else if (census.isContributor(reviewer)) {
role = "contributor";
} else {
throw new IllegalStateException("No role for reviewer: " + reviewer);
}

roles.put(reviewer, role);
}

var missing = new HashMap<String, Integer>(requirements);
for (var reviewer : reviewers) {
var role = roles.get(reviewer);
if (missing.get(role) == 0) {
var next = nextRequiredRole(role, missing);
if (next != null) {
missing.put(next, missing.get(next) - 1);
}
} else {
missing.put(role, missing.get(role) - 1);
}
}

for (var role : missing.keySet()) {
int required = requirements.get(role);
int n = missing.get(role);
if (n > 0) {
log.finer("issue: too few reviewers with role " + role + " found");
return iterator(new TooFewReviewersIssue(required - n, required, role, metadata));
}
}

var username = commit.author().name();
var email = commit.author().email();
if (email != null && email.endsWith("@" + domain)) {
@@ -28,24 +28,42 @@
import java.util.stream.Collectors;

public class ReviewersConfiguration {
static final ReviewersConfiguration DEFAULT = new ReviewersConfiguration(1, "reviewer", List.of("duke"));
static final ReviewersConfiguration DEFAULT = new ReviewersConfiguration(0, 1, 0, 0, 0, List.of("duke"));

private final int minimum;
private final String role;
private final int lead;
private final int reviewers;
private final int committers;
private final int authors;
private final int contributors;
private final List<String> ignore;

ReviewersConfiguration(int minimum, String role, List<String> ignore) {
this.minimum = minimum;
this.role = role;
ReviewersConfiguration(int lead, int reviewers, int committers, int authors, int contributors, List<String> ignore) {
this.lead = lead;
this.reviewers = reviewers;
this.committers = committers;
this.authors = authors;
this.contributors = contributors;
this.ignore = ignore;
}

public int minimum() {
return minimum;
public int lead() {
return lead;
}

public String role() {
return role;
public int reviewers() {
return reviewers;
}

public int committers() {
return committers;
}

public int authors() {
return authors;
}

public int contributors() {
return contributors;
}

public List<String> ignore() {
@@ -61,9 +79,36 @@ static ReviewersConfiguration parse(Section s) {
return DEFAULT;
}

var minimum = s.get("minimum", DEFAULT.minimum());
var role = s.get("role", DEFAULT.role());
var lead = s.get("lead", DEFAULT.lead());
var reviewers = s.get("reviewers", DEFAULT.reviewers());
var committers = s.get("committers", DEFAULT.committers());
var authors = s.get("authors", DEFAULT.authors());
var contributors = s.get("contributors", DEFAULT.contributors());

if (s.contains("minimum")) {
var minimum = s.get("minimum").asInt();
if (s.contains("role")) {
var role = s.get("role").asString();
if (role.equals("lead")) {
lead = minimum;
} else if (role.equals("reviewer")) {
reviewers = minimum;
} else if (role.equals("committer")) {
committers = minimum;
} else if (role.equals("author")) {
authors = minimum;
} else if (role.equals("contributor")) {
contributors = minimum;
} else {
throw new IllegalArgumentException("Unexpected role: " + role);
}
} else {
reviewers = minimum;
}
}

var ignore = s.get("ignore", DEFAULT.ignore());
return new ReviewersConfiguration(minimum, role, ignore);

return new ReviewersConfiguration(lead, reviewers, committers, authors, contributors, ignore);
}
}
@@ -25,11 +25,13 @@
public class TooFewReviewersIssue extends CommitIssue {
private final int numActual;
private final int numRequired;
private final String role;

TooFewReviewersIssue(int numActual, int numRequired, CommitIssue.Metadata metadata) {
TooFewReviewersIssue(int numActual, int numRequired, String role, CommitIssue.Metadata metadata) {
super(metadata);
this.numActual = numActual;
this.numRequired = numRequired;
this.role = role;
}

public int numRequired() {
@@ -40,6 +42,10 @@ public int numActual() {
return numActual;
}

public String role() {
return role;
}

@Override
public void accept(IssueVisitor v) {
v.visit(this);

0 comments on commit daac177

Please sign in to comment.