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: report all issues from "committer" check #532

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
@@ -27,6 +27,7 @@
import org.openjdk.skara.vcs.Commit;
import org.openjdk.skara.vcs.openjdk.CommitMessage;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.logging.Logger;

@@ -58,6 +59,7 @@ private boolean hasRole(Project project, String role, String username, int versi

@Override
Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration conf) {
var issues = new ArrayList<Issue>();
var project = census.project(conf.general().project());
var version = conf.census().version();
var domain = conf.census().domain();
@@ -67,23 +69,26 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
var committer = commit.committer();
if (committer.name() == null || committer.name().isEmpty()) {
log.finer("issue: committer.name is null or empty");
return iterator(new CommitterNameIssue(metadata));
issues.add(new CommitterNameIssue(metadata));
}
if (committer.email() == null || !committer.email().endsWith("@" + domain)) {
log.finer("issue: committer.email is null or does not end with @" + domain);
return iterator(new CommitterEmailIssue(domain, metadata));
issues.add(new CommitterEmailIssue(domain, metadata));
}

var username = committer.email().split("@")[0];
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));
if (committer.name() != null || committer.email() != null) {
var username = committer.email() == null ?
committer.name() : committer.email().split("@")[0];
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);
issues.add(new CommitterIssue(project, metadata));
}
}
}

return iterator();
return issues.iterator();
}

@Override
@@ -210,7 +210,7 @@ void missingCommitterNameShouldFail() throws IOException {
var check = new CommitterCheck(census());
var issues = toList(check.check(commit, message, conf()));

assertEquals(1, issues.size());
assertEquals(2, issues.size());
assertTrue(issues.get(0) instanceof CommitterNameIssue);
var issue = (CommitterNameIssue) issues.get(0);
assertEquals(commit, issue.commit());
@@ -228,7 +228,7 @@ void missingCommitterEmailShouldFail() throws IOException {
var check = new CommitterCheck(census());
var issues = toList(check.check(commit, message, conf()));

assertEquals(1, issues.size());
assertEquals(2, issues.size());
assertTrue(issues.get(0) instanceof CommitterEmailIssue);
var issue = (CommitterEmailIssue) issues.get(0);
assertEquals(commit, issue.commit());