Repository navigation
Conversation
JsonSyncable is now created through a factory registered in AddCrdtDataCore, so its constructor can take internal services. This removes the public JsonSyncable type from the package; callers use IJsonSyncableFactory.Create(dir), which returns an ISyncable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The static SyncHelper becomes SyncService, a singleton registered in AddCrdtDataCore and injected into DataModel and JsonSyncable (via its factory), so it can take a logger and config later. It takes the serializer options in its constructor instead of per call. No behavior change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 53 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughSynchronization now uses an injected Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 7 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: replacing static synchronization with an injectable SyncService. Description check The description directly explains the SyncService injection, JsonSyncable API change, factory usage, registration, and intended lack of behavior change. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
myieye
changed the title
…vice Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
[Claude, autonomous]
Turn the static
SyncHelperinto an injectableSyncService, so the logging follow-up can give it a logger and config. No behavior change.JsonSyncableis now internal. Create one withIJsonSyncableFactory.Create(dir)(registered byAddCrdtData), which returns anISyncable; its constructor andFilenamePrefix/FilenameExtensionconstants are no longer public. Nothing in Lexbox uses them.SyncWithResourceUploadmoved over as is; it still has no callers.🤖 Generated with Claude Code