Skip to content

Commit

Permalink
2272: Improve issuestitleCheck
Browse files Browse the repository at this point in the history
Reviewed-by: erikj
  • Loading branch information
zhaosongzs committed Jun 3, 2024
1 parent 54ca259 commit 53601bb
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3446,7 +3446,7 @@ void issuesTitleCheck(TestInfo testInfo) throws IOException {
var issue2 = issues.createIssue("this is an issue2 etc.", List.of("Hello"), Map.of());
pr.addComment("/issue add " + issue2.id());
TestBotRunner.runPeriodicItems(prBot);
assertTrue(pr.store().body().contains("Found trailing period in issue title for `2: this is an issue2 etc.`"));
assertFalse(pr.store().body().contains("Found trailing period in issue title for `2: this is an issue2 etc.`"));
assertTrue(pr.store().body().contains("Found leading lowercase letter in issue title for `2: this is an issue2 etc.`"));

// Change the leading letter to upper case
Expand All @@ -3455,7 +3455,7 @@ void issuesTitleCheck(TestInfo testInfo) throws IOException {
TestBotRunner.runPeriodicItems(prBot);
// The additional issue marker should be updated, so the warning of leading lowercase letter no longer exists
assertFalse(pr.store().body().contains("Found leading lowercase letter in issue title for `2: this is an issue2 etc.`"));
assertTrue(pr.store().body().contains("Found trailing period in issue title for `2: This is an issue2 etc.`"));
assertFalse(pr.store().body().contains("Found trailing period in issue title for `2: This is an issue2 etc.`"));

// Approve it as Reviewer, warnings shouldn't prevent adding ready label to the pr
var reviewerPr = reviewer.pullRequest(pr.id());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.logging.Logger;
import java.util.regex.Pattern;

public class IssuesTitleCheck extends CommitCheck {
private final Logger log = Logger.getLogger("org.openjdk.skara.jcheck.issuesTitle");
private final static List<String> VALID_WORD_WITH_TRAILING_PERIOD = List.of("et al.", "etc.", "...");
private final static Pattern FILE_OR_FUNCTION_PATTERN = Pattern.compile(".*[()/._:].*");

@Override
Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration conf, Census census) {
Expand All @@ -48,10 +52,10 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
var issuesWithLeadingLowerCaseLetter = new ArrayList<String>();

for (var issue : message.issues()) {
if (issue.description().endsWith(".")) {
if (hasTrailingPeriod(issue.description())) {
issuesWithTrailingPeriod.add("`" + issue + "`");
}
if (Character.isLowerCase(issue.description().charAt(0))) {
if (hasLeadingLowerCaseLetter(issue.description())) {
issuesWithLeadingLowerCaseLetter.add("`" + issue + "`");
}
}
Expand All @@ -71,4 +75,24 @@ public String description() {
return "Issue's title should be properly formatted";
}

private boolean hasTrailingPeriod(String description) {
if (!description.endsWith(".")) {
return false;
}
for (String phrase : VALID_WORD_WITH_TRAILING_PERIOD) {
if (description.endsWith(phrase)) {
return false;
}
}
return true;
}

private boolean hasLeadingLowerCaseLetter(String description) {
if (Character.isUpperCase(description.charAt(0))) {
return false;
}
var firstWord = description.split(" ")[0];
// If first word contains special character, it's very likely a reference to file or function, ignore it
return !FILE_OR_FUNCTION_PATTERN.matcher(firstWord).matches();
}
}

0 comments on commit 53601bb

Please sign in to comment.