diff --git a/cli/src/main/java/org/openjdk/skara/cli/GitJCheck.java b/cli/src/main/java/org/openjdk/skara/cli/GitJCheck.java index ab3b44150..0cdc4f20f 100644 --- a/cli/src/main/java/org/openjdk/skara/cli/GitJCheck.java +++ b/cli/src/main/java/org/openjdk/skara/cli/GitJCheck.java @@ -322,7 +322,8 @@ public static int run(Repository repo, String[] args) throws IOException { } } - var visitor = new JCheckCLIVisitor(ignore, isMercurial); + var isLax = getSwitch("lax", arguments); + var visitor = new JCheckCLIVisitor(ignore, isMercurial, isLax); lines = repo.config("jcheck.pre-push.commits"); var shouldCheckCommits = lines.size() == 1 && lines.get(0).toLowerCase().equals("true"); var commitMessageParser = isMercurial ? CommitMessageParsers.v0 : CommitMessageParsers.v1; diff --git a/cli/src/main/java/org/openjdk/skara/cli/JCheckCLIVisitor.java b/cli/src/main/java/org/openjdk/skara/cli/JCheckCLIVisitor.java index 0615f46cf..6bb1a6930 100644 --- a/cli/src/main/java/org/openjdk/skara/cli/JCheckCLIVisitor.java +++ b/cli/src/main/java/org/openjdk/skara/cli/JCheckCLIVisitor.java @@ -33,15 +33,17 @@ class JCheckCLIVisitor implements IssueVisitor { private final Set ignore; private final boolean isMercurial; + private final boolean isLax; private boolean hasDisplayedErrors; public JCheckCLIVisitor() { - this(Set.of(), false); + this(Set.of(), false, false); } - public JCheckCLIVisitor(Set ignore, boolean isMercurial) { + public JCheckCLIVisitor(Set ignore, boolean isMercurial, boolean isLax) { this.ignore = ignore; this.isMercurial = isMercurial; + this.isLax = isLax; this.hasDisplayedErrors = false; } @@ -76,7 +78,7 @@ public void visit(DuplicateIssuesIssue i) { } public void visit(TagIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { println(i, "illegal tag name: " + i.tag().name()); hasDisplayedErrors = true; } @@ -90,14 +92,14 @@ public void visit(BranchIssue i) { } public void visit(SelfReviewIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { println(i, "self-reviews are not allowed"); hasDisplayedErrors = true; } } public void visit(TooFewReviewersIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { var required = i.numRequired(); var actual = i.numActual(); var reviewers = required == 1 ? " reviewer" : " reviewers"; @@ -116,14 +118,14 @@ public void visit(InvalidReviewersIssue i) { } public void visit(MergeMessageIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { println(i, "merge commits should only use the commit message '" + i.expected() + "'"); hasDisplayedErrors = true; } } public void visit(HgTagCommitIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { hasDisplayedErrors = true; switch (i.error()) { case TOO_MANY_LINES: @@ -199,7 +201,7 @@ private static List ranges(List errors) } public void visit(WhitespaceIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { var pos = i.path() + ":" + i.row(); var prefix = println(i, i.describe() + " in " + pos); var indent = prefix.replaceAll(".", " "); @@ -210,7 +212,7 @@ public void visit(WhitespaceIssue i) { } public void visit(MessageIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { println(i, "contains additional lines in commit message"); for (var line : i.message().additional()) { System.out.println("> " + line); @@ -220,7 +222,7 @@ public void visit(MessageIssue i) { } public void visit(MessageWhitespaceIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { String desc = null; if (i.kind().isTab()) { desc = "tab"; @@ -236,7 +238,7 @@ public void visit(MessageWhitespaceIssue i) { } public void visit(IssuesIssue i) { - if (!ignore.contains(i.check().name())) { + if (!ignore.contains(i.check().name()) && !isLax) { println(i, "missing reference to JBS issue in commit message"); for (var line : i.commit().message()) { System.out.println("> " + line); diff --git a/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesCheck.java b/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesCheck.java index fe224e2f0..14da3366f 100644 --- a/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesCheck.java +++ b/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesCheck.java @@ -44,7 +44,8 @@ Iterator check(Commit commit, CommitMessage message, JCheckConfiguration } var metadata = CommitIssue.metadata(commit, message, conf, this); - if (commit.message().isEmpty() || message.issues().isEmpty()) { + if (conf.checks().issues().required() && + (commit.message().isEmpty() || message.issues().isEmpty())) { log.finer("issue: no reference to a JBS issue"); return iterator(new IssuesIssue(metadata)); } diff --git a/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesConfiguration.java b/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesConfiguration.java index a2d1f88f4..d7b7469a4 100644 --- a/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesConfiguration.java +++ b/jcheck/src/main/java/org/openjdk/skara/jcheck/IssuesConfiguration.java @@ -30,18 +30,24 @@ public class IssuesConfiguration { static final IssuesConfiguration DEFAULT = - new IssuesConfiguration("^(([A-Z][A-Z0-9]+-)?[0-9]+): (\\S.*)$"); + new IssuesConfiguration("^(([A-Z][A-Z0-9]+-)?[0-9]+): (\\S.*)$", true); private final String pattern; + private final boolean required; - IssuesConfiguration(String pattern) { + IssuesConfiguration(String pattern, boolean required) { this.pattern = pattern; + this.required = required; } public String pattern() { return pattern; } + public boolean required() { + return required; + } + static String name() { return "issues"; } @@ -52,6 +58,7 @@ static IssuesConfiguration parse(Section s) { } var pattern = s.get("pattern", DEFAULT.pattern()); - return new IssuesConfiguration(pattern); + var required = s.get("required", DEFAULT.required()); + return new IssuesConfiguration(pattern, required); } } diff --git a/jcheck/src/main/java/org/openjdk/skara/jcheck/JCheckConfiguration.java b/jcheck/src/main/java/org/openjdk/skara/jcheck/JCheckConfiguration.java index 81f85163e..0e6bdd72e 100644 --- a/jcheck/src/main/java/org/openjdk/skara/jcheck/JCheckConfiguration.java +++ b/jcheck/src/main/java/org/openjdk/skara/jcheck/JCheckConfiguration.java @@ -73,13 +73,19 @@ private static INI convert(INI old) { config.add("jbs=JDK"); config.add("[checks]"); - var error = "error=blacklist,author,committer,reviewers,merge,hg-tag,message,issues,executable,symlink"; + var error = "error=blacklist,author,committer,reviewers,merge,issues,executable,symlink"; var shouldCheckWhitespace = false; var checkWhitespace = old.get("whitespace"); if (checkWhitespace == null || !checkWhitespace.asString().equals("lax")) { error += ",whitespace"; shouldCheckWhitespace = true; } + var shouldCheckMessage = false; + var checkMessage = old.get("comments"); + if (checkMessage == null || !checkMessage.asString().equals("lax")) { + error += ",message,hg-tag"; + shouldCheckMessage = true; + } config.add(error); if (project.startsWith("jdk")) { @@ -118,7 +124,11 @@ private static INI convert(INI old) { config.add("message=Merge"); config.add("[checks \"reviewers\"]"); - config.add("contributors=1"); + if (shouldCheckMessage) { + config.add("contributors=1"); + } else { + config.add("contributors=0"); + } config.add("ignore=duke"); config.add("[checks \"committer\"]"); @@ -126,6 +136,9 @@ private static INI convert(INI old) { config.add("[checks \"issues\"]"); config.add("pattern=^([124-8][0-9]{6}): (\\S.*)$"); + if (!shouldCheckMessage) { + config.add("required = false"); + } return INI.parse(config); }