Skip to content

Commit

Permalink
Add small description of each category in notifications
Browse files Browse the repository at this point in the history
  • Loading branch information
yrodiere committed Dec 11, 2024
1 parent 9d0e198 commit 8f40f01
Show file tree
Hide file tree
Showing 13 changed files with 190 additions and 61 deletions.
41 changes: 11 additions & 30 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 Down Expand Up @@ -228,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 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
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,7 +1,9 @@
package io.quarkus.github.lottery.message;

import java.time.temporal.Temporal;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import jakarta.enterprise.context.ApplicationScoped;
Expand All @@ -28,6 +30,9 @@ public class MessageFormatter {
private static final String PAYLOAD_BEGIN = "<!--:payload:\n";
private static final String PAYLOAD_END = "\n:payload:-->";

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

@Inject
ObjectMapper jsonObjectMapper;

Expand Down Expand Up @@ -103,6 +108,14 @@ static String repositoryName(DrawRef drawRef) {
return drawRef.repositoryRef().repositoryName();
}

static String asMarkdownLabel(String label) {
return "`" + label + "`";
}

static String asMarkdownLabel(Collection<String> labels) {
return labels.stream().map(TemplateExtensions::asMarkdownLabel).collect(Collectors.joining("/"));
}

}

@TemplateExtension(namespace = "github")
Expand Down
44 changes: 36 additions & 8 deletions src/main/resources/templates/MessageFormatter/notificationBody.md
Original file line number Diff line number Diff line change
@@ -1,25 +1,53 @@
Hey @{report.username}, here's your report for {report.repositoryName} on {report.localDate}.
{#if !report.config.maintenanceLabels.isEmpty}

Maintenance areas: {report.config.maintenanceLabels.asMarkdownLabel}.
{/if}

{#if report.triage.present}
# Triage
{#include MessageFormatter/notificationBodyBucketContent bucket=report.triage.get() /}
{#include MessageFormatter/notificationBodyBucketContent bucket=report.triage.get()}

<sup>Issues that haven't been assigned an area yet. Please add an area label, remove the {report.config.triageLabel.asMarkdownLabel} label, optionally ping maintainers.</sup>

{/include}

{/if}
{#if report.feedbackNeeded.present}
# Feedback needed (reproducer, information, ...)
{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackNeeded.get() /}
# Feedback needed
{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackNeeded.get()}

<sup>Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long.</sup>

{/include}

{/if}
{#if report.feedbackProvided.present}
# Feedback provided (reproducer, information, ...)
{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackProvided.get() /}
# Feedback provided
{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackProvided.get()}

<sup>Issues with newly provided reproducer/information. Please have a closer look, possibly remove the {report.config.feedbackLabels.asMarkdownLabel} label, and plan further work.</sup>

{/include}

{/if}
{#if report.stale.present}
# Stale
{#include MessageFormatter/notificationBodyBucketContent bucket=report.stale.get() /}
{#include MessageFormatter/notificationBodyBucketContent bucket=report.stale.get()}

<sup>Issues last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ...</sup>

{/include}

{/if}
{#if report.stewardship.present}
# Stewardship
{#include MessageFormatter/notificationBodyBucketContent bucket=report.stewardship.get() /}
{/if}
{#include MessageFormatter/notificationBodyBucketContent bucket=report.stewardship.get()}

<sup>Issues across all areas last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ...</sup>

{/include}

{/if}
---
<sup>If you no longer want to receive these notifications, just close [any issue assigned to you in the notification repository](https://github.com/{notificationRepositoryName}/issues/assigned/@me). Reopening the issue will resume the notifications.</sup>
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{@io.quarkus.github.lottery.draw.LotteryReport$Bucket bucket}
{#if bucket.issues.isEmpty}

No issues in this category this time.
{#else}
{#insert /}
{#for issue in bucket.issues}
- [#{issue.number}]({issue.url}) {issue.title}
{/for}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

Expand Down
Loading

0 comments on commit 8f40f01

Please sign in to comment.