Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jcheck: make merge committers configurable #494

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -75,9 +75,12 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
}

var username = committer.email().split("@")[0];
if (!hasRole(project, role, username, version)) {
log.finer("issue: committer does not have role " + role);
return iterator(new CommitterIssue(project, metadata));
var allowedToMerge = conf.checks().committer().allowedToMerge();
if (!commit.isMerge() || !allowedToMerge.contains(username)) {
if (!hasRole(project, role, username, version)) {
log.finer("issue: committer does not have role " + role);
return iterator(new CommitterIssue(project, metadata));
}
}

return iterator();
@@ -24,22 +24,30 @@

import org.openjdk.skara.ini.Section;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class CommitterConfiguration {
static final CommitterConfiguration DEFAULT = new CommitterConfiguration("committer");
static final CommitterConfiguration DEFAULT = new CommitterConfiguration("committer", Set.of());

private final String role;
private final Set<String> allowedToMerge;

CommitterConfiguration(String role) {
CommitterConfiguration(String role, Set<String> allowedToMerge) {
this.role = role;
this.allowedToMerge = allowedToMerge;
}

public String role() {
return role;
}

public Set<String> allowedToMerge() {
return allowedToMerge;
}

static String name() {
return "committer";
}
@@ -50,7 +58,12 @@ static CommitterConfiguration parse(Section s) {
}

var role = s.get("role", DEFAULT.role());
return new CommitterConfiguration(role);
var usernames = s.get("allowed-to-merge", "");
var allowedToMerge = new HashSet<String>();
for (var username : usernames.split(",")) {
allowedToMerge.add(username.trim());
}
return new CommitterConfiguration(role, allowedToMerge);
}
}

@@ -75,6 +75,15 @@ class CommitterCheckTests {
"error = committer"
);

private static Commit mergeCommit(Author author, Author committer) {
var hash = new Hash("0123456789012345678901234567890123456789");
var parents = List.of(hash, hash);
var date = ZonedDateTime.now();
var message = List.of("Merge");
var metadata = new CommitMetadata(hash, parents, author, committer, date, message);
return new Commit(metadata, List.of());
}

private static Commit commit(Author author, Author committer) {
var hash = new Hash("0123456789012345678901234567890123456789");
var parents = List.of(new Hash("12345789012345789012345678901234567890"));
@@ -227,4 +236,40 @@ void missingCommitterEmailShouldFail() throws IOException {
assertEquals(message, issue.message());
assertEquals(Severity.ERROR, issue.severity());
}

@Test
void allowedToMerge() throws IOException {
var author = new Author("baz", "baz@localhost");
var committer = new Author("baz", "baz@localhost");
var commit = mergeCommit(author, committer);
var message = message(commit);
var check = new CommitterCheck(census());
var issues = toList(check.check(commit, message, conf()));

assertEquals(1, issues.size());
assertTrue(issues.get(0) instanceof CommitterIssue);

check = new CommitterCheck(census());
var text = new ArrayList<>(CONFIGURATION);
text.addAll(List.of("[checks \"committer\"]", "allowed-to-merge=baz"));
var conf = JCheckConfiguration.parse(text);
issues = toList(check.check(commit, message, conf));
assertEquals(List.of(), issues);
}

@Test
void allowedToMergeShouldOnlyWorkForMergeCommits() throws IOException {
var author = new Author("baz", "baz@localhost");
var committer = new Author("baz", "baz@localhost");
var commit = commit(author, committer);
var message = message(commit);
var check = new CommitterCheck(census());
var text = new ArrayList<>(CONFIGURATION);
text.addAll(List.of("[checks \"committer\"]", "allowed-to-merge=baz"));
var conf = JCheckConfiguration.parse(text);
var issues = toList(check.check(commit, message, conf));

assertEquals(1, issues.size());
assertTrue(issues.get(0) instanceof CommitterIssue);
}
}