Skip to content

Commit ffa6562

Browse files
ayagmarayagmar
committed
quarkusio#469 Update Editorial bot comments on edit
1 parent 3c4873e commit ffa6562

18 files changed

+1003
-155
lines changed

‎src/main/java/io/quarkus/bot/CheckIssueEditorialRules.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,23 @@
11
package io.quarkus.bot;
22

3-
import java.io.IOException;
4-
5-
import jakarta.inject.Inject;
6-
7-
import org.jboss.logging.Logger;
8-
import org.kohsuke.github.GHEventPayload;
9-
import org.kohsuke.github.GHIssue;
10-
113
import io.quarkiverse.githubapp.ConfigFile;
124
import io.quarkiverse.githubapp.event.Issue;
135
import io.quarkus.bot.config.Feature;
146
import io.quarkus.bot.config.QuarkusGitHubBotConfig;
157
import io.quarkus.bot.config.QuarkusGitHubBotConfigFile;
168
import io.quarkus.bot.util.Strings;
9+
import jakarta.inject.Inject;
10+
import org.jboss.logging.Logger;
11+
import org.kohsuke.github.GHEventPayload;
12+
import org.kohsuke.github.GHIssue;
13+
14+
import java.io.IOException;
1715

1816
public class CheckIssueEditorialRules {
1917
private static final Logger LOG = Logger.getLogger(CheckIssueEditorialRules.class);
2018

2119
private static final String ZULIP_URL = "https://quarkusio.zulipchat.com/";
22-
public static final String ZULIP_WARNING = Strings.commentByBot(
20+
public static final String ZULIP_WARNING = Strings.editorialCommentByBot(
2321
"You added a link to a Zulip discussion, please make sure the description of the issue is comprehensive and doesn't require accessing Zulip");
2422

2523
@Inject

‎src/main/java/io/quarkus/bot/CheckPullRequestEditorialRules.java

Lines changed: 48 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,39 @@
11
package io.quarkus.bot;
22

33
import java.io.IOException;
4-
import java.util.ArrayList;
5-
import java.util.Arrays;
6-
import java.util.Collections;
74
import java.util.List;
8-
import java.util.Locale;
9-
import java.util.regex.Pattern;
5+
import java.util.Optional;
6+
import java.util.StringJoiner;
107

118
import jakarta.inject.Inject;
129

13-
import org.jboss.logging.Logger;
10+
import org.apache.commons.lang3.StringUtils;
1411
import org.kohsuke.github.GHEventPayload;
12+
import org.kohsuke.github.GHIssueComment;
1513
import org.kohsuke.github.GHPullRequest;
1614

1715
import io.quarkiverse.githubapp.ConfigFile;
1816
import io.quarkiverse.githubapp.event.PullRequest;
1917
import io.quarkus.bot.config.Feature;
20-
import io.quarkus.bot.config.QuarkusGitHubBotConfig;
2118
import io.quarkus.bot.config.QuarkusGitHubBotConfigFile;
2219
import io.quarkus.bot.util.GHPullRequests;
23-
import io.quarkus.bot.util.PullRequestFilesMatcher;
24-
import io.quarkus.bot.util.Strings;
20+
import io.quarkus.bot.violation.EditorialViolation;
21+
import io.quarkus.bot.violation.ViolationDetectorManager;
2522

2623
class CheckPullRequestEditorialRules {
2724

28-
private static final Logger LOG = Logger.getLogger(CheckPullRequestEditorialRules.class);
25+
public static final String HEADER = """
26+
Thanks for your pull request!
2927
30-
private static final Pattern SPACE_PATTERN = Pattern.compile("\\s+");
31-
private static final Pattern ISSUE_PATTERN = Pattern.compile("#[0-9]+");
32-
private static final Pattern FIX_FEAT_CHORE = Pattern.compile("^(fix|chore|feat|docs|refactor)[(:].*");
28+
Your pull request does not follow our editorial rules. Could you have a look?
3329
34-
private static final List<String> UPPER_CASE_EXCEPTIONS = Arrays.asList("gRPC");
35-
private static final List<String> BOMS = List.of("bom/application/pom.xml");
36-
private static final List<String> DOC_CHANGES = List.of("docs/src/main/asciidoc/", "README.md", "LICENSE",
37-
"CONTRIBUTING.md");
30+
""";
3831

3932
@Inject
40-
QuarkusGitHubBotConfig quarkusBotConfig;
33+
GitHubBotActions gitHubBotActions;
34+
35+
@Inject
36+
ViolationDetectorManager violationManager;
4137

4238
void checkPullRequestEditorialRules(@PullRequest.Opened GHEventPayload.PullRequest pullRequestPayload,
4339
@ConfigFile("quarkus-github-bot.yml") QuarkusGitHubBotConfigFile quarkusBotConfigFile) throws IOException {
@@ -48,122 +44,65 @@ void checkPullRequestEditorialRules(@PullRequest.Opened GHEventPayload.PullReque
4844
String baseBranch = pullRequestPayload.getPullRequest().getBase().getRef();
4945

5046
GHPullRequest pullRequest = pullRequestPayload.getPullRequest();
51-
String body = pullRequest.getBody();
5247
String originalTitle = pullRequest.getTitle();
5348
String normalizedTitle = GHPullRequests.normalizeTitle(originalTitle, baseBranch);
5449

5550
if (!originalTitle.equals(normalizedTitle)) {
56-
pullRequest.setTitle(normalizedTitle);
51+
gitHubBotActions.setPullRequestTitle(pullRequest, normalizedTitle);
5752
}
5853

59-
// we remove the potential version prefix before checking the editorial rules
60-
String title = GHPullRequests.dropVersionSuffix(normalizedTitle, baseBranch);
61-
62-
List<String> titleErrorMessages = getTitleErrorMessages(title);
63-
List<String> bodyErrorMessages = getBodyErrorMessages(body, pullRequest);
54+
List<EditorialViolation> violations = violationManager.detectViolations(pullRequest);
55+
List<String> titleErrorMessages = violationManager.getTitleViolationMessages(violations);
56+
List<String> bodyErrorMessages = violationManager.getBodyViolationMessages(violations);
6457

6558
if (titleErrorMessages.isEmpty() && bodyErrorMessages.isEmpty()) {
6659
return;
6760
}
68-
69-
StringBuilder comment = new StringBuilder("""
70-
Thanks for your pull request!
71-
72-
Your pull request does not follow our editorial rules. Could you have a look?
73-
74-
""");
61+
StringJoiner comment = new StringJoiner("\n", HEADER, StringUtils.EMPTY);
7562
for (String errorMessage : titleErrorMessages) {
76-
comment.append("- ").append(errorMessage).append("\n");
63+
comment.add("- " + errorMessage);
7764
}
7865
for (String errorMessage : bodyErrorMessages) {
79-
comment.append("- ").append(errorMessage).append("\n");
66+
comment.add("- " + errorMessage);
8067
}
8168

82-
if (!quarkusBotConfig.isDryRun()) {
83-
pullRequest.comment(Strings.commentByBot(comment.toString()));
84-
} else {
85-
LOG.info("Pull request #" + pullRequest.getNumber() + " - Add comment " + comment.toString());
86-
}
69+
gitHubBotActions.addComment(pullRequest, comment.toString(), true);
8770
}
8871

89-
private static List<String> getTitleErrorMessages(String title) {
90-
List<String> errorMessages = new ArrayList<>();
91-
92-
if (title == null || title.isEmpty()) {
93-
return Collections.singletonList("title should not be empty");
94-
}
9572

96-
if (title.endsWith(".")) {
97-
errorMessages.add("title should not end up with dot");
98-
}
99-
if (title.endsWith("…")) {
100-
errorMessages.add("title should not end up with ellipsis (make sure the title is complete)");
101-
}
102-
if (SPACE_PATTERN.split(title.trim()).length < 2) {
103-
errorMessages.add("title should count at least 2 words to describe the change properly");
104-
}
105-
if (!Character.isDigit(title.codePointAt(0)) && !Character.isUpperCase(title.codePointAt(0))
106-
&& !isUpperCaseException(title)) {
107-
errorMessages.add("title should preferably start with an uppercase character (if it makes sense!)");
108-
}
109-
if (ISSUE_PATTERN.matcher(title).find()) {
110-
errorMessages.add("title should not contain an issue number (use `Fix #1234` in the description instead)");
111-
}
112-
if (FIX_FEAT_CHORE.matcher(title).matches()) {
113-
errorMessages.add("title should not start with chore/docs/feat/fix/refactor but be a proper sentence");
73+
void checkPullRequestEditorialRulesOnEdit(@PullRequest.Edited GHEventPayload.PullRequest pullRequestPayload,
74+
@ConfigFile("quarkus-github-bot.yml") QuarkusGitHubBotConfigFile quarkusBotConfigFile) throws IOException {
75+
if (!Feature.CHECK_EDITORIAL_RULES.isEnabled(quarkusBotConfigFile)) {
76+
return;
11477
}
78+
var pullRequest = pullRequestPayload.getPullRequest();
11579

116-
return errorMessages;
117-
}
118-
119-
private static List<String> getBodyErrorMessages(String body, GHPullRequest pullRequest) throws IOException {
120-
List<String> errorMessages = new ArrayList<>();
80+
// Detect violations using the violation manager
81+
List<EditorialViolation> violations = violationManager.detectViolations(pullRequest);
82+
var titleErrorMessages = violationManager.getTitleViolationMessages(violations);
83+
var bodyErrorMessages = violationManager.getBodyViolationMessages(violations);
12184

122-
if ((body == null || body.isBlank()) && isMeaningfulPullRequest(pullRequest)) {
123-
return List.of(
124-
"description should not be empty, describe your intent or provide links to the issues this PR is fixing (using `Fixes #NNNNN`) or changelogs");
125-
}
85+
if (titleErrorMessages.isEmpty() && bodyErrorMessages.isEmpty()) {
86+
Optional<GHIssueComment> commentToDeleteOpt = gitHubBotActions.findBotComment(pullRequest);
87+
commentToDeleteOpt
88+
.ifPresent(ghIssueComment -> gitHubBotActions.deleteComment(ghIssueComment, pullRequest.getNumber()));
89+
} else {
90+
Optional<GHIssueComment> existingCommentOpt = gitHubBotActions.findBotComment(pullRequest);
12691

127-
return errorMessages;
128-
}
92+
StringJoiner comment = new StringJoiner("\n", HEADER, "");
93+
for (String errorMessage : titleErrorMessages) {
94+
comment.add("- " + errorMessage);
95+
}
96+
for (String errorMessage : bodyErrorMessages) {
97+
comment.add("- " + errorMessage);
98+
}
12999

130-
private static boolean isUpperCaseException(String title) {
131-
for (String exception : UPPER_CASE_EXCEPTIONS) {
132-
if (title.toLowerCase(Locale.ROOT).startsWith(exception.toLowerCase(Locale.ROOT))) {
133-
return true;
100+
if (existingCommentOpt.isPresent()) {
101+
gitHubBotActions.updateComment(existingCommentOpt.get(), comment.toString(), pullRequest.getNumber(), true);
102+
} else {
103+
gitHubBotActions.addComment(pullRequest, comment.toString(), true);
134104
}
135105
}
136-
137-
return false;
138106
}
139107

140-
private static boolean isMeaningfulPullRequest(GHPullRequest pullRequest) throws IOException {
141-
// Note: these rules will have to be adjusted depending on how it goes
142-
// we don't want to annoy people fixing a typo or require a description for a one liner explained in the title
143-
144-
// if we have more than one commit, then it's meaningful
145-
if (pullRequest.getCommits() > 1) {
146-
return true;
147-
}
148-
149-
PullRequestFilesMatcher filesMatcher = new PullRequestFilesMatcher(pullRequest);
150-
151-
// for changes to the BOM, we are stricter
152-
if (filesMatcher.changedFilesMatch(BOMS)) {
153-
return true;
154-
}
155-
156-
// for one liner/two liners, let's be a little more lenient
157-
if (pullRequest.getAdditions() <= 2 && pullRequest.getDeletions() <= 2) {
158-
return false;
159-
}
160-
161-
// let's be a little more flexible for doc changes
162-
if (filesMatcher.changedFilesOnlyMatch(DOC_CHANGES)
163-
&& pullRequest.getAdditions() <= 10 && pullRequest.getDeletions() <= 10) {
164-
return false;
165-
}
166-
167-
return true;
168-
}
169108
}

‎src/main/java/io/quarkus/bot/GitHubBotActions.java

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package io.quarkus.bot;
2+
3+
import static io.quarkus.bot.util.Strings.EDITORIAL_RULES_COMMENT_MARKER;
4+
5+
import java.io.IOException;
6+
import java.util.List;
7+
import java.util.Optional;
8+
9+
import jakarta.enterprise.context.RequestScoped;
10+
import jakarta.inject.Inject;
11+
12+
import org.jboss.logging.Logger;
13+
import org.kohsuke.github.GHIssueComment;
14+
import org.kohsuke.github.GHPullRequest;
15+
16+
import io.quarkus.bot.config.QuarkusGitHubBotConfig;
17+
import io.quarkus.bot.util.Strings;
18+
19+
/**
20+
* This class handles all GitHub operations related to bot actions.
21+
* It provides methods for adding, updating, and removing comments on pull requests,
22+
* as well as setting pull request titles.
23+
*/
24+
@RequestScoped
25+
class GitHubBotActions {
26+
27+
private static final Logger LOG = Logger.getLogger(GitHubBotActions.class);
28+
29+
@Inject
30+
QuarkusGitHubBotConfig quarkusBotConfig;
31+
32+
33+
public void addComment(GHPullRequest pullRequest, String comment, boolean isEditorialComment) {
34+
if (!quarkusBotConfig.isDryRun()) {
35+
try {
36+
String formattedComment = isEditorialComment ? Strings.editorialCommentByBot(comment)
37+
: Strings.commentByBot(comment);
38+
pullRequest.comment(formattedComment);
39+
LOG.debug("Pull request #" + pullRequest.getNumber() + " - Added new comment");
40+
} catch (IOException e) {
41+
LOG.error(String.format("Pull Request #%s - Failed to add comment", pullRequest.getNumber()), e);
42+
}
43+
} else {
44+
LOG.info("Pull request #" + pullRequest.getNumber() + " - Add comment " + comment);
45+
}
46+
}
47+
48+
49+
public void updateComment(GHIssueComment comment, String newText, int pullRequestNumber, boolean isEditorialComment) {
50+
if (!quarkusBotConfig.isDryRun()) {
51+
try {
52+
String formattedComment = isEditorialComment ? Strings.editorialCommentByBot(newText)
53+
: Strings.commentByBot(newText);
54+
comment.update(formattedComment);
55+
LOG.debug("Pull request #" + pullRequestNumber + " - Updated comment");
56+
} catch (IOException e) {
57+
LOG.error(String.format("Pull Request #%s - Failed to update comment %d",
58+
pullRequestNumber, comment.getId()), e);
59+
}
60+
} else {
61+
LOG.info(String.format("Pull Request #%s - Update comment %d with: %s",
62+
pullRequestNumber, comment.getId(), newText));
63+
}
64+
}
65+
66+
67+
public void deleteComment(GHIssueComment comment, int pullRequestNumber) {
68+
if (!quarkusBotConfig.isDryRun()) {
69+
try {
70+
comment.delete();
71+
LOG.debug("Pull request #" + pullRequestNumber + " - Deleted comment");
72+
} catch (IOException e) {
73+
LOG.error(String.format("Pull Request #%s - Failed to delete comment %d",
74+
pullRequestNumber, comment.getId()), e);
75+
}
76+
} else {
77+
LOG.info(String.format("Pull Request #%s - Delete comment %d",
78+
pullRequestNumber, comment.getId()));
79+
}
80+
}
81+
82+
83+
public void setPullRequestTitle(GHPullRequest pullRequest, String newTitle) {
84+
if (!quarkusBotConfig.isDryRun()) {
85+
try {
86+
pullRequest.setTitle(newTitle);
87+
LOG.debug("Pull request #" + pullRequest.getNumber() + " - Updated title to: " + newTitle);
88+
} catch (IOException e) {
89+
LOG.error(String.format("Pull Request #%s - Failed to update title", pullRequest.getNumber()), e);
90+
}
91+
} else {
92+
LOG.info("Pull request #" + pullRequest.getNumber() + " - Update title to: " + newTitle);
93+
}
94+
}
95+
96+
97+
public Optional<GHIssueComment> findBotComment(GHPullRequest pullRequest) throws IOException {
98+
List<GHIssueComment> comments = pullRequest.getComments();
99+
if (comments == null || comments.isEmpty()) {
100+
return Optional.empty();
101+
}
102+
103+
return comments.stream()
104+
.filter(comment -> comment.getBody() != null)
105+
.filter(comment -> comment.getBody().contains(EDITORIAL_RULES_COMMENT_MARKER))
106+
.findFirst();
107+
}
108+
}

‎src/main/java/io/quarkus/bot/util/Strings.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.quarkus.bot.util;
22

33
public class Strings {
4+
public static final String EDITORIAL_RULES_COMMENT_MARKER = "\n<!-- Quarkus-Bot-Editor-Rules -->";
45

56
public static boolean isNotBlank(String string) {
67
return string != null && !string.trim().isEmpty();
@@ -19,6 +20,10 @@ public static String commentByBot(String originalComment) {
1920
return sb.toString();
2021
}
2122

23+
public static String editorialCommentByBot(String originalComment) {
24+
return commentByBot(originalComment) + EDITORIAL_RULES_COMMENT_MARKER;
25+
}
26+
2227
private Strings() {
2328
}
2429
}

0 commit comments

Comments
 (0)