Skip to content

Commit

Permalink
Merge pull request #196 from yrodiere/i180-i31
Browse files Browse the repository at this point in the history
 Allow ignoring issues by labels for stale/stewardship +  Add small description of each category in notifications
  • Loading branch information
yrodiere authored Dec 11, 2024
2 parents 2f20db0 + 8f40f01 commit 0b7abd7
Show file tree
Hide file tree
Showing 18 changed files with 432 additions and 107 deletions.
55 changes: 24 additions & 31 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,10 @@ See below for details about each section.
[[participants-triage]]
=== Triage

If the `triage` section is present, you will get notified when new issues are created
and need to be routed to the right person/team.
If the `triage` section is present, you will get notified about issues that haven't been assigned an area yet.

Addressing these notifications generally involves removing the `triage/needs-triage` label,
adding the correct `+area/*+` labels, and optionally pinging the relevant maintainers
(though the https://github.com/quarkusio/quarkus-github-bot#triage-issues[Quarkus GitHub Bot should generally take care of that part]
once you add `+area/*+` labels).
Please add an area label, remove the "needs triage" (e.g. `triage/needs-triage`) label,
and ping the relevant maintainers if necessary.

Of course, some more discussion might be necessary before that, and that's fine:
issues that don't change will reappear in another notification, a few days later.
Expand Down Expand Up @@ -141,30 +138,17 @@ that may be stalled and require intervention from maintainers or reporters.
Issues in "maintenance" notifications will be split in three categories:

Feedback Needed::
These issues have the `triage/needs-reproducer` label,
and it looks like the Quarkus team was the last to comment on the issue,
quite some time ago.
Issues with missing reproducer/information.
+
Depending on the actual content of the issue, you might want to:
+
* send a (gentle!) reminder to the reporter that we need feedback (a reproducer, more information, ...) before we can do anything.
* or, if it's really been too long, close the issue because we cannot work on it.
Please ping the reporter, or close the issue if it's taking too long.
Feedback Provided::
These issues have the `triage/needs-reproducer` label,
and it looks like someone who is not from the Quarkus team was the last to comment on the issue,
quite some time ago.
Issues with newly provided reproducer/information.
+
There might be feedback (a reproducer, more information, ...) there,
in which case you might want to remove the `triage/needs-reproducer` label
and have a closer look.
Please have a closer look, possibly remove the "needs feedback" (e.g. `triage/needs-reproducer`) label, and plan further work.
Stale::
These issues have not been updated for a very long time.
+
Depending on the actual content of the issue, you might want to:
Issues last updated a long time ago.
+
* prioritize the issue and work on it soon;
* or send a reminder to someone you've been waiting on;
* or close the issue because it's no longer relevant.
Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ...

Of course, in every situation, simply continuing the conversation,
pinging someone, or even doing nothing at all are perfectly acceptable responses:
Expand All @@ -187,11 +171,12 @@ participants:
maxIssues: 2
stale:
maxIssues: 5
ignoreLabels: ["triage/on-ice"]
----

`labels`::
The labels identifying issues you are interested in, as a maintainer.
Issues mentioned in notifications will have at least one of those labels.
Issues mentioned in notifications will have at least one of these labels.
+
Array of Strings, mandatory, no default.
`days`::
Expand All @@ -213,6 +198,11 @@ How many issues, at most, you wish to be included in the "Stale" category
for each notification.
+
Integer, mandatory, no default.
`stale.ignoreLabels`::
The labels identifying issues that should be ignored for the "Stale" category.
Issues mentioned in notifications will never have any one of these labels.
+
Array of Strings, optional, defaults to an empty array.

[[participants-stewardship]]
=== Stewardship
Expand All @@ -222,12 +212,9 @@ core contributors who spend significant time working on GitHub issues.
If you don't already know what this section is about,
you probably don't want to use it.

If the `stewardship` section is present, you will get notified about issues that just became stale,
across the whole project, without any consideration for the labels assigned to those issues.
If the `stewardship` section is present, you will get notified about Issues across all areas last updated a long time ago.

Depending on the actual content of the issue, you might want to simply continue the conversation,
ping someone, close the issue, or even do nothing at all:
it's all up to you, and issues that don't change will reappear in another notification, a few days later.
Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ...

NOTE: Notifications to stewards are sent independently of notifications to maintainers,
so that the work of maintainers won't be affected by the work of stewards.
Expand All @@ -244,6 +231,7 @@ participants:
stewardship:
days: ["MONDAY"]
maxIssues: 5
ignoreLabels: ["triage/on-ice"]
----

`days`::
Expand All @@ -255,6 +243,11 @@ How many issues, at most, you wish to be included in the "stewardship" category
for each notification.
+
Integer, mandatory, no default.
`ignoreLabels`::
The labels identifying issues that should be ignored for the "stewardship" category.
Issues mentioned in notifications will never have any one of these labels.
+
Array of Strings, optional, defaults to an empty array.

[[participants-suspending]]
=== Suspending notifications
Expand Down
13 changes: 8 additions & 5 deletions src/main/java/io/quarkus/github/lottery/LotteryService.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Optional;

Expand All @@ -16,13 +17,13 @@
import io.quarkus.github.lottery.config.LotteryConfig;
import io.quarkus.github.lottery.draw.DrawRef;
import io.quarkus.github.lottery.draw.Lottery;
import io.quarkus.github.lottery.draw.Participant;
import io.quarkus.github.lottery.history.LotteryHistory;
import io.quarkus.github.lottery.draw.LotteryReport;
import io.quarkus.github.lottery.draw.Participant;
import io.quarkus.github.lottery.github.GitHubRepository;
import io.quarkus.github.lottery.github.GitHubRepositoryRef;
import io.quarkus.github.lottery.github.GitHubService;
import io.quarkus.github.lottery.history.HistoryService;
import io.quarkus.github.lottery.history.LotteryHistory;
import io.quarkus.github.lottery.notification.NotificationService;
import io.quarkus.github.lottery.notification.Notifier;
import io.quarkus.logging.Log;
Expand Down Expand Up @@ -89,7 +90,7 @@ private void doDrawForRepository(GitHubRepository repo, LotteryConfig lotteryCon

lottery.draw(repo, history);

var sent = notifyParticipants(notifier, participants);
var sent = notifyParticipants(lotteryConfig, notifier, participants);
if (!sent.isEmpty()) {
try {
historyService.append(drawRef, lotteryConfig, sent);
Expand Down Expand Up @@ -130,10 +131,12 @@ private List<Participant> registerParticipants(DrawRef drawRef, Lottery lottery,
return participants;
}

private List<LotteryReport.Serialized> notifyParticipants(Notifier notifier, List<Participant> participants) {
private List<LotteryReport.Serialized> notifyParticipants(LotteryConfig lotteryConfig,
Notifier notifier, List<Participant> participants) {
List<LotteryReport.Serialized> sent = new ArrayList<>();
for (var participant : participants) {
var report = participant.report();
var report = participant.report(lotteryConfig.buckets().triage().label(),
new LinkedHashSet<>(lotteryConfig.buckets().maintenance().feedback().labels()));
try {
Log.debugf("Sending report: %s", report);
notifier.send(report);
Expand Down
25 changes: 19 additions & 6 deletions src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ public record LotteryConfig(

public static final String FILE_NAME = "quarkus-github-lottery.yml";

public LotteryConfig(
@JsonProperty(required = true) Notifications notifications,
@JsonProperty(required = true) Buckets buckets,
List<Participant> participants) {
this.notifications = notifications;
this.buckets = buckets;
this.participants = participants == null ? List.of() : participants;
}

public record Notifications(
@JsonProperty(required = true) CreateIssuesConfig createIssues) {
public record CreateIssuesConfig(
Expand Down Expand Up @@ -81,23 +90,27 @@ public Provided(@JsonProperty(required = true) Duration delay,
}

public record Stale(
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification) {
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification,
@JsonProperty(required = true) List<String> ignoreLabels) {
// https://stackoverflow.com/a/71539100/6692043
// Also gives us a less verbose constructor for tests
@JsonCreator
public Stale(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout) {
this(new Notification(delay, timeout));
public Stale(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout,
@JsonProperty(required = false) List<String> ignoreLabels) {
this(new Notification(delay, timeout), ignoreLabels == null ? List.of() : ignoreLabels);
}
}
}

public record Stewardship(
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification) {
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification,
@JsonProperty(required = false) List<String> ignoreLabels) {
// https://stackoverflow.com/a/71539100/6692043
// Also gives us a less verbose constructor for tests
@JsonCreator
public Stewardship(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout) {
this(new Notification(delay, timeout));
public Stewardship(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout,
@JsonProperty(required = false) List<String> ignoreLabels) {
this(new Notification(delay, timeout), ignoreLabels == null ? List.of() : ignoreLabels);
}
}

Expand Down
9 changes: 6 additions & 3 deletions src/main/java/io/quarkus/github/lottery/draw/Lottery.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List<Draw
String label = config.triage().label();
var cutoff = now.minus(config.triage().notification().delay());
var history = lotteryHistory.triage();
draws.add(triage.bucket.createDraw(repo.issuesWithLabelLastUpdatedBefore(label, cutoff)
draws.add(triage.bucket.createDraw(repo.issuesWithLabelLastUpdatedBefore(label, Set.of(), cutoff)
.filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number()))
.iterator(),
allWinnings));
Expand Down Expand Up @@ -155,9 +155,11 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List<Draw
}
if (stale.hasParticipation()) {
var cutoff = now.minus(config.maintenance().stale().notification().delay());
// Remove duplicates, but preserve order
var ignoreLabels = new LinkedHashSet<>(config.maintenance().stale().ignoreLabels());
var history = lotteryHistory.stale();
draws.add(stale.createDraw(
repo.issuesWithLabelLastUpdatedBefore(areaLabel, cutoff)
repo.issuesWithLabelLastUpdatedBefore(areaLabel, ignoreLabels, cutoff)
.filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number()))
.iterator(),
allWinnings));
Expand All @@ -176,9 +178,10 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List<Draw
Set<Integer> allWinnings) throws IOException {
if (stewardship.bucket.hasParticipation()) {
var cutoff = now.minus(config.stewardship().notification().delay());
var ignoreLabels = new LinkedHashSet<>(config.stewardship().ignoreLabels());
var history = lotteryHistory.stewardship();
draws.add(bucket.createDraw(
repo.issuesLastUpdatedBefore(cutoff)
repo.issuesLastUpdatedBefore(ignoreLabels, cutoff)
.filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number()))
.iterator(),
allWinnings));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.time.ZoneId;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -17,6 +18,7 @@ public record LotteryReport(
DrawRef drawRef,
String username,
Optional<ZoneId> timezone,
Config config,
Optional<Bucket> triage,
Optional<Bucket> feedbackNeeded,
Optional<Bucket> feedbackProvided,
Expand All @@ -33,6 +35,12 @@ public boolean hasContent() {
return buckets().anyMatch(Bucket::hasContent);
}

public record Config(
String triageLabel,
Set<String> feedbackLabels,
Set<String> maintenanceLabels) {
}

public record Bucket(
List<Issue> issues) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,12 @@ public void participate(Lottery lottery) {
stewardship.ifPresent(lottery.stewardship()::participate);
}

public LotteryReport report() {
public LotteryReport report(String triageLabel, Set<String> feedbackLabels) {
return new LotteryReport(drawRef, username, timezone,
new LotteryReport.Config(
triageLabel,
feedbackLabels,
maintenance.map(m -> m.labels).orElseGet(Set::of)),
triage.map(Participation::issues).map(LotteryReport.Bucket::new),
maintenance.flatMap(m -> m.feedbackNeeded).map(Participation::issues).map(LotteryReport.Bucket::new),
maintenance.flatMap(m -> m.feedbackProvided).map(Participation::issues).map(LotteryReport.Bucket::new),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
package io.quarkus.github.lottery.github;

import static io.quarkus.github.lottery.github.GitHubSearchClauses.anyLabel;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.assignee;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.author;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.isIssue;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.label;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.not;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.repo;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.updatedBefore;
import static io.quarkus.github.lottery.util.Streams.toStream;
import static io.quarkus.github.lottery.util.UncheckedIOFunction.uncheckedIO;

Expand Down Expand Up @@ -95,8 +103,8 @@ private GHRepository repository() throws IOException {

private GHIssueSearchBuilder searchIssues() {
return client().searchIssues()
.q(GitHubSearchClauses.repo(ref))
.q(GitHubSearchClauses.isIssue());
.q(repo(ref))
.q(isIssue());
}

private DynamicGraphQLClient graphQLClient() {
Expand All @@ -114,37 +122,43 @@ public Optional<LotteryConfig> fetchLotteryConfig() throws IOException {
/**
* Lists issues that were last updated before the given instant.
*
* @param ignoreLabels GitHub labels. Issues assigned with any of these labels are ignored (not returned).
* @param updatedBefore An instant; all returned issues must have been last updated before that instant.
* @return A lazily populated stream of matching issues.
* @throws java.io.UncheckedIOException In case of I/O failure.
*/
public Stream<Issue> issuesLastUpdatedBefore(Instant updatedBefore) {
return toStream(searchIssues()
public Stream<Issue> issuesLastUpdatedBefore(Set<String> ignoreLabels, Instant updatedBefore) {
var builder = searchIssues()
.isOpen()
.q(GitHubSearchClauses.updatedBefore(updatedBefore))
.q(updatedBefore(updatedBefore))
.sort(GHIssueSearchBuilder.Sort.UPDATED)
.order(GHDirection.DESC)
.list())
.map(toIssueRecord());
.order(GHDirection.DESC);
if (!ignoreLabels.isEmpty()) {
builder.q(not(anyLabel(ignoreLabels)));
}
return toStream(builder.list()).map(toIssueRecord());
}

/**
* Lists issues with the given label that were last updated before the given instant.
*
* @param label A GitHub label; if non-null, all returned issues must have been assigned that label.
* @param ignoreLabels GitHub labels. Issues assigned with any of these labels are ignored (not returned).
* @param updatedBefore An instant; all returned issues must have been last updated before that instant.
* @return A lazily populated stream of matching issues.
* @throws java.io.UncheckedIOException In case of I/O failure.
*/
public Stream<Issue> issuesWithLabelLastUpdatedBefore(String label, Instant updatedBefore) {
return toStream(searchIssues()
public Stream<Issue> issuesWithLabelLastUpdatedBefore(String label, Set<String> ignoreLabels, Instant updatedBefore) {
var builder = searchIssues()
.isOpen()
.q(GitHubSearchClauses.label(label))
.q(GitHubSearchClauses.updatedBefore(updatedBefore))
.q(label(label))
.q(updatedBefore(updatedBefore))
.sort(GHIssueSearchBuilder.Sort.UPDATED)
.order(GHDirection.DESC)
.list())
.map(toIssueRecord());
.order(GHDirection.DESC);
if (!ignoreLabels.isEmpty()) {
builder.q(not(anyLabel(ignoreLabels)));
}
return toStream(builder.list()).map(toIssueRecord());
}

/**
Expand All @@ -163,9 +177,9 @@ public Stream<Issue> issuesLastActedOnByAndLastUpdatedBefore(Set<String> initial
IssueActionSide lastActionSide, Instant updatedBefore) {
return toStream(searchIssues()
.isOpen()
.q(GitHubSearchClauses.anyLabel(initialActionLabels))
.q(GitHubSearchClauses.label(filterLabel))
.q(GitHubSearchClauses.updatedBefore(updatedBefore))
.q(anyLabel(initialActionLabels))
.q(label(filterLabel))
.q(updatedBefore(updatedBefore))
.sort(GHIssueSearchBuilder.Sort.UPDATED)
.order(GHDirection.DESC)
.list())
Expand Down Expand Up @@ -305,9 +319,9 @@ public void update(String topicSuffix, String markdownBody, boolean comment)

private Stream<GHIssue> getDedicatedIssues() throws IOException {
var builder = searchIssues()
.q(GitHubSearchClauses.author(appLogin()));
.q(author(appLogin()));
if (ref.assignee() != null) {
builder.q(GitHubSearchClauses.assignee(ref.assignee()));
builder.q(assignee(ref.assignee()));
}
return toStream(builder.list())
.filter(ref.expectedSuffixStart() != null
Expand Down
Loading

0 comments on commit 0b7abd7

Please sign in to comment.