Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughMinor SDK updates: normalize POINTS→POINT in Ecency point-transfer op; adjust cache keys/invalidation for favourites, relations, follow/unfollow; reduce trending-tags query staleTime to 1 hour; bump package versions and changelogs. Changes
Sequence Diagram(s)(Skipped — changes are small, localized, and do not introduce a new multi-component control flow that benefits from a sequence diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/sdk/src/modules/operations/builders/ecency.ts (1)
149-150: Optional: preferreplaceAll(or add thegflag) for completeness.
String.prototype.replacewith a non-global regex replaces only the first match. For a well-formed amount like"1.000 POINTS"there is at most one occurrence, so this is not a bug today. However, usingreplaceAllis slightly more defensive and communicates intent more clearly:♻️ Suggested simplification
- const normalizedAmount = amount.replace(/POINTS\b/, "POINT"); + const normalizedAmount = amount.replaceAll("POINTS", "POINT");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/modules/operations/builders/ecency.ts` around lines 149 - 150, The normalization currently uses amount.replace(/POINTS\b/, "POINT") which only replaces the first match; update the normalization for normalizedAmount in the ecency builder to be global/explicit by using either amount.replaceAll("POINTS", "POINT") or amount.replace(/POINTS\b/g, "POINT") so all occurrences are normalized and intent is clear (reference: normalizedAmount variable in packages/sdk/src/modules/operations/builders/ecency.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/sdk/src/modules/operations/builders/ecency.ts`:
- Around line 149-150: The normalization currently uses
amount.replace(/POINTS\b/, "POINT") which only replaces the first match; update
the normalization for normalizedAmount in the ecency builder to be
global/explicit by using either amount.replaceAll("POINTS", "POINT") or
amount.replace(/POINTS\b/g, "POINT") so all occurrences are normalized and
intent is clear (reference: normalizedAmount variable in
packages/sdk/src/modules/operations/builders/ecency.ts).
Summary by CodeRabbit
Bug Fixes
Improvements
Chores