Conversation
WalkthroughThis update adds a new reminder service that regularly checks which GitHub reviewers need a nudge and sends them a message in the appropriate Discord thread. It introduces a new record and enum to track review details and statuses. The data model now stores extra info like review status, thread ID, and last reminder time. Several new methods and classes support these reminders and improve how mentions are managed. Some existing code was adjusted to use the new data and services, and unused imports were cleaned up. The reminder service starts with the app and stops gracefully on shutdown. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/main/java/com/eternalcode/discordapp/review/GitHubReviewReminderService.java (5)
45-57: Addthis.prefix to fix checkstyle issues.The shutdown logic looks good, just need to add
this.beforeschedulerreferences.public void stop() { - scheduler.shutdown(); + this.scheduler.shutdown(); try { - if (!scheduler.awaitTermination(60, TimeUnit.SECONDS)) { - scheduler.shutdownNow(); + if (!this.scheduler.awaitTermination(60, TimeUnit.SECONDS)) { + this.scheduler.shutdownNow(); } } catch (InterruptedException e) { - scheduler.shutdownNow(); + this.scheduler.shutdownNow(); Thread.currentThread().interrupt(); } LOGGER.info("GitHub review reminder service stopped"); }🧰 Tools
🪛 GitHub Actions: Java CI with Gradle
[error] 46-46: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 48-48: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 49-49: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 53-53: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
59-73: Addthis.prefix for checkstyle compliance.The error handling looks good. Just need the prefix fixes.
private void sendReminders() { try { - mentionRepository.getReviewersNeedingReminders(reminderInterval) + this.mentionRepository.getReviewersNeedingReminders(this.reminderInterval) .thenAccept(this::processReminders) .exceptionally(throwable -> { Sentry.captureException(throwable); LOGGER.log(Level.SEVERE, "Error sending reminders", throwable); return null; }); } catch (Exception exception) { Sentry.captureException(exception); LOGGER.log(Level.SEVERE, "Error scheduling reminders", exception); } }🧰 Tools
🪛 GitHub Actions: Java CI with Gradle
[error] 61-61: Checkstyle: Reference to instance variable 'mentionRepository' needs "this.". (RequireThis)
[error] 61-61: Checkstyle: Reference to instance variable 'reminderInterval' needs "this.". (RequireThis)
75-79: Fix checkstyle issue with method call.Add
this.prefix to the method call.private void processReminders(List<GitHubReviewMentionRepository.ReviewerReminder> reminders) { for (GitHubReviewMentionRepository.ReviewerReminder reminder : reminders) { - sendReminder(reminder); + this.sendReminder(reminder); } }🧰 Tools
🪛 GitHub Actions: Java CI with Gradle
[error] 77-77: Checkstyle: Method call to 'sendReminder' needs "this.". (RequireThis)
81-104: Fix checkstyle issues and consider tracking failed reminders.Add
this.prefix and maybe track when users or threads can't be found.private void sendReminder(GitHubReviewMentionRepository.ReviewerReminder reminder) { long userId = reminder.userId(); String pullRequestUrl = reminder.pullRequestUrl(); long threadId = reminder.threadId(); - jda.retrieveUserById(userId).queue( + this.jda.retrieveUserById(userId).queue( user -> { if (user == null) { LOGGER.warning("Could not find user with ID " + userId); return; } - ThreadChannel thread = jda.getThreadChannelById(threadId); + ThreadChannel thread = this.jda.getThreadChannelById(threadId); if (thread == null) { LOGGER.warning("Could not find thread with ID " + threadId); return; } - sendReminderMessage(user, thread, pullRequestUrl); + this.sendReminderMessage(user, thread, pullRequestUrl); }, throwable -> { Sentry.captureException(throwable); LOGGER.log(Level.SEVERE, "Error retrieving user", throwable); }); }🧰 Tools
🪛 GitHub Actions: Java CI with Gradle
[error] 86-86: Checkstyle: Reference to instance variable 'jda' needs "this.". (RequireThis)
[error] 93-93: Checkstyle: Reference to instance variable 'jda' needs "this.". (RequireThis)
[error] 99-99: Checkstyle: Method call to 'sendReminderMessage' needs "this.". (RequireThis)
106-125: Addthis.prefix for checkstyle.The reminder logic looks good. Just need the prefix fix.
thread.sendMessage(message).queue( success -> { GitHubPullRequest pullRequest = GitHubPullRequest.fromUrl(pullRequestUrl).orNull(); if (pullRequest != null) { - mentionRepository.recordReminderSent(pullRequest, user.getIdLong()); + this.mentionRepository.recordReminderSent(pullRequest, user.getIdLong()); } }, throwable -> { Sentry.captureException(throwable); LOGGER.log(Level.SEVERE, "Error sending reminder message", throwable); } );🧰 Tools
🪛 GitHub Actions: Java CI with Gradle
[error] 117-117: Checkstyle: Reference to instance variable 'mentionRepository' needs "this.". (RequireThis)
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionWrapper.java (1)
49-63: Factory method has good defaults but lacks validation.The factory method correctly sets
lastReminderSentto 0 for new mentions, but consider adding null checks for the input parameters.public static GitHubReviewMentionWrapper create( String pullRequest, long userId, Instant lastMention, GitHubReviewStatus reviewStatus, long threadId) { + if (pullRequest == null || lastMention == null || reviewStatus == null) { + throw new IllegalArgumentException("Required parameters cannot be null"); + } return new GitHubReviewMentionWrapper( pullRequest, userId, lastMention.toEpochMilli(), reviewStatus.name(), threadId, 0 ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/eternalcode/discordapp/DiscordApp.java(4 hunks)src/main/java/com/eternalcode/discordapp/review/GitHubReviewMention.java(1 hunks)src/main/java/com/eternalcode/discordapp/review/GitHubReviewReminderService.java(1 hunks)src/main/java/com/eternalcode/discordapp/review/GitHubReviewService.java(2 hunks)src/main/java/com/eternalcode/discordapp/review/GitHubReviewStatus.java(1 hunks)src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionRepository.java(1 hunks)src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionRepositoryImpl.java(3 hunks)src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionWrapper.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionRepository.java (1)
src/main/java/com/eternalcode/discordapp/review/GitHubReviewMention.java (1)
GitHubReviewMention(3-49)
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionRepositoryImpl.java (1)
src/main/java/com/eternalcode/discordapp/review/GitHubReviewMention.java (1)
GitHubReviewMention(3-49)
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionWrapper.java (1)
src/main/java/com/eternalcode/discordapp/review/GitHubReviewMention.java (1)
GitHubReviewMention(3-49)
🪛 GitHub Actions: Java CI with Gradle
src/main/java/com/eternalcode/discordapp/review/GitHubReviewMention.java
[error] 27-27: Checkstyle: Reference to instance variable 'pullRequest' needs "this.". (RequireThis)
[error] 31-31: Checkstyle: Reference to instance variable 'userId' needs "this.". (RequireThis)
[error] 35-35: Checkstyle: Reference to instance variable 'lastMention' needs "this.". (RequireThis)
[error] 39-39: Checkstyle: Reference to instance variable 'reviewStatus' needs "this.". (RequireThis)
[error] 43-43: Checkstyle: Reference to instance variable 'threadId' needs "this.". (RequireThis)
[error] 47-47: Checkstyle: Reference to instance variable 'lastReminderSent' needs "this.". (RequireThis)
src/main/java/com/eternalcode/discordapp/review/GitHubReviewReminderService.java
[error] 41-41: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 46-46: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 48-48: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 49-49: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 53-53: Checkstyle: Reference to instance variable 'scheduler' needs "this.". (RequireThis)
[error] 61-61: Checkstyle: Reference to instance variable 'mentionRepository' needs "this.". (RequireThis)
[error] 61-61: Checkstyle: Reference to instance variable 'reminderInterval' needs "this.". (RequireThis)
[error] 77-77: Checkstyle: Method call to 'sendReminder' needs "this.". (RequireThis)
[error] 86-86: Checkstyle: Reference to instance variable 'jda' needs "this.". (RequireThis)
[error] 93-93: Checkstyle: Reference to instance variable 'jda' needs "this.". (RequireThis)
[error] 99-99: Checkstyle: Method call to 'sendReminderMessage' needs "this.". (RequireThis)
[error] 117-117: Checkstyle: Reference to instance variable 'mentionRepository' needs "this.". (RequireThis)
🔇 Additional comments (13)
src/main/java/com/eternalcode/discordapp/review/GitHubReviewService.java (2)
26-27: Formatting improvement looks good.The multi-line format improves readability for the string constant.
145-145: Method signature update is correct.Adding the forumId parameter aligns with the repository interface changes for tracking thread associations.
src/main/java/com/eternalcode/discordapp/review/GitHubReviewStatus.java (1)
3-31: Well-designed enum implementation.The status values cover all relevant GitHub review states, and the fromString method provides flexible parsing with a sensible default fallback.
src/main/java/com/eternalcode/discordapp/DiscordApp.java (2)
73-73: Good approach using static field for shared repository.This allows both GitHubReviewService and GitHubReviewReminderService to use the same repository instance.
186-191: Proper reminder service integration.The service is correctly initialized and includes a shutdown hook for graceful cleanup.
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionRepository.java (2)
16-23: Nice API design!The new methods and ReviewerReminder record look clean and consistent with the async pattern.
12-12:Details
✅ Verification successful
Check that all callers are updated with the new threadId parameter.
This is a breaking change to the interface. Make sure all implementations and callers pass the threadId.
🏁 Script executed:
#!/bin/bash # Description: Find all calls to markReviewerAsMentioned to verify they're updated # Search for method calls rg -A 2 "markReviewerAsMentioned\(" --type javaLength of output: 415
All callers updated with the new threadId parameter
- We found a single call in
src/main/java/com/eternalcode/discordapp/review/GitHubReviewService.java
and it now passesforumIdas thethreadId.- No other
markReviewerAsMentioned(usages were detected.Looks all set!
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionWrapper.java (6)
3-4: Good addition of required imports.The new imports for
GitHubReviewStatusandGitHubReviewMentionare correctly added to support the enhanced functionality.
9-9: Table name change looks intentional.The table name change from "review_mentions" to "github_review_mentions" provides better clarity about the table's purpose.
21-28: New database fields are properly configured.The three new fields are correctly annotated with
@DatabaseFieldand use appropriate data types for their purposes.
34-46: Constructor properly handles new parameters.The private constructor correctly initializes all new fields with the provided parameters.
89-95: Smart null handling for reminder timestamps.The getter and setter methods handle the "never sent" state elegantly by using 0 as a sentinel value and converting to/from null.
97-106: Conversion method correctly maps all fields.The
toMention()method properly creates aGitHubReviewMentionobject using the raw stored values, which matches the expected constructor parameters.
src/main/java/com/eternalcode/discordapp/review/GitHubReviewMention.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/discordapp/review/GitHubReviewReminderService.java
Show resolved
Hide resolved
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionRepositoryImpl.java
Show resolved
Hide resolved
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionRepositoryImpl.java
Show resolved
Hide resolved
src/main/java/com/eternalcode/discordapp/review/database/GitHubReviewMentionWrapper.java
Show resolved
Hide resolved
Signed-off-by: Martin Sulikowski <vLuckyyy.biznes@gmail.com>
Signed-off-by: Martin Sulikowski <vLuckyyy.biznes@gmail.com>
1 participant
No description provided.