refactor: extract clampLimit helper and remove redundant optional chaining#714
refactor: extract clampLimit helper and remove redundant optional chaining#714kamilwronka wants to merge 1 commit intodevelopfrom
Conversation
…ining - Extract duplicated `Math.min(Math.max(limit, 1), 50)` into a private `clampLimit` method in ActivitiesQueryService (used 3 times in same file) - Remove redundant `.getTime?.()` optional chaining in find-active-event-heroes-by-npc.ts — if `createdAt` is a Date, `.getTime` is always defined, so `.getTime?.()` is unnecessary Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo small refactoring changes: extraction of duplicate clamping logic into a shared helper method within the activities query service, and correction of optional chaining syntax for Date.getTime() invocation in event hero sorting. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
Summary
clampLimithelper inActivitiesQueryService: the expressionMath.min(Math.max(limit, 1), 50)was duplicated 3 times acrosssuggestActorNames,suggestWorlds, andsuggestClanNames. Extracted into a privateclampLimitmethod..getTime?.()optional chaining infind-active-event-heroes-by-npc.ts: whencreatedAtis aDate,.getTimeis always defined — the?.on the method call was unnecessary. Changed.getTime?.()to.getTime().Files touched
apps/activity/src/activities/services/activities-query.service.tsapps/api/src/events/utils/find-active-event-heroes-by-npc.tsSafety
Test plan
oxfmt --check)oxlint)🤖 Generated with Claude Code
Summary by CodeRabbit