Repository navigation
refactor: extract shared IntervalScheduler base class (#1694) - #1752
Conversation
📝 WalkthroughWalkthroughAdds a shared ChangesScheduler lifecycle refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Failed to generate code suggestions for PR |
Extract the common start/stop/tick scaffold from BirthdayScheduler, ReminderScheduler, and GiveawayScheduler into a reusable IntervalScheduler base class. This eliminates lifecycle duplication and allows removal of both reminderScheduler and giveawayScheduler from cpd.exclusions, as the code duplication is now resolved. The base class manages: - timer and client lifecycle - tickInProgress guard for concurrent execution prevention - start/stop methods for lifecycle management - tick() method with error handling Subclasses override execute() for domain-specific logic and onStart() for custom initialization (e.g., running tick immediately on startup). Fixes #1694
7a62c66 to
9c46d69
Compare
IntervalScheduler extraction left an extra closing brace in reminderScheduler.ts and birthdayScheduler.ts, closing the class body before deliverReminder()/reconcileGuildsWithoutMatches() etc — Jest failed to parse both files (SyntaxError). Also dedents the over-indented execute() bodies (leftover from removing the old while-loop wrapper) and drops the now-unused Client import (client access moved to the IntervalScheduler base class).
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Refactors scheduler lifecycle into a base class, which is high-impact code reorganization that could affect timing and concurrency.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bot/src/utils/general/giveawayScheduler.ts (1)
11-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
clockinjection
GiveawaySchedulernever readsthis.clock, so the constructor option has no effect. Remove the field/option, or thread it intoexecute()if time-based behavior is planned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/utils/general/giveawayScheduler.ts` around lines 11 - 18, Remove the unused clock field and clock constructor option from GiveawayScheduler; keep the constructor focused on tickIntervalMs unless execute() is updated to use the injected clock.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/bot/src/utils/general/giveawayScheduler.ts`:
- Around line 11-18: Remove the unused clock field and clock constructor option
from GiveawayScheduler; keep the constructor focused on tickIntervalMs unless
execute() is updated to use the injected clock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33b59243-e654-4438-96ad-220cf6525498
📒 Files selected for processing (5)
packages/bot/src/utils/general/IntervalScheduler.tspackages/bot/src/utils/general/birthdayScheduler.tspackages/bot/src/utils/general/giveawayScheduler.tspackages/bot/src/utils/general/reminderScheduler.tssonar-project.properties
|



Summary
Extract the common start/stop/tick scaffold from BirthdayScheduler, ReminderScheduler, and GiveawayScheduler into a reusable
IntervalSchedulerbase class that manages:timer,clientfields)tickInProgressguard)Subclasses implement
execute()for domain-specific logic and overrideonStart()for custom initialization (e.g., running tick immediately on startup for birthday/reminder schedulers).Resolves code duplication and allows removal of both reminderScheduler and giveawayScheduler from
sonar-project.propertiescpd.exclusions.Changes
IntervalSchedulerbase class with protected lifecycle methodsTest Plan
Closes #1694
Summary by cubic
Refactored the birthday, reminder, and giveaway schedulers to share a new
IntervalSchedulerbase, removing duplicated lifecycle code and standardizing error handling. Re-enabled Sonar CPD checks by removing scheduler exclusions (closes #1694).Refactors
IntervalSchedulerwith start/stop, concurrency guard, and centralized error handling.onStart()for startup work andexecute()for periodic tasks; birthday/reminder run an immediate tick on start.packages/bot/src/utils/general/reminderScheduler.tsand.../giveawayScheduler.tsfromsonar.cpd.exclusions.Bug Fixes
execute()bodies after refactor.Clientimports.Written for commit 9e0c2ab. Summary will update on new commits.
Summary by CodeRabbit