diff --git a/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterCheck.java b/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterCheck.java index db79a67a4..a6c3f4406 100644 --- a/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterCheck.java +++ b/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterCheck.java @@ -75,9 +75,12 @@ Iterator 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(); diff --git a/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterConfiguration.java b/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterConfiguration.java index 612d0dbc6..7993ded21 100644 --- a/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterConfiguration.java +++ b/jcheck/src/main/java/org/openjdk/skara/jcheck/CommitterConfiguration.java @@ -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 allowedToMerge; - CommitterConfiguration(String role) { + CommitterConfiguration(String role, Set allowedToMerge) { this.role = role; + this.allowedToMerge = allowedToMerge; } public String role() { return role; } + public Set 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(); + for (var username : usernames.split(",")) { + allowedToMerge.add(username.trim()); + } + return new CommitterConfiguration(role, allowedToMerge); } } diff --git a/jcheck/src/test/java/org/openjdk/skara/jcheck/CommitterCheckTests.java b/jcheck/src/test/java/org/openjdk/skara/jcheck/CommitterCheckTests.java index 5f435e399..613f448ed 100644 --- a/jcheck/src/test/java/org/openjdk/skara/jcheck/CommitterCheckTests.java +++ b/jcheck/src/test/java/org/openjdk/skara/jcheck/CommitterCheckTests.java @@ -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); + } }