Repository navigation
Split sense API out of CrdtMiniLcmApi - #2733
Conversation
Move the sense methods into CrdtSenseApi. Following how entry links went to their reference-type services, AddSemanticDomainToSense and RemoveSemanticDomainFromSense move to CrdtSemanticDomainsApi and SetSensePartOfSpeech to CrdtPartsOfSpeechApi. CreateSenseChanges and VerifySenseBelongsToEntry become internal statics on CrdtSenseApi, since CreateEntry and the example sentence methods still use them from the facade. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change adds Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to No concrete issue remains that would block merging after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. A rabbit checks each sense in line Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
AI summary
Stacked on #2732. Moves the sense methods out of
CrdtMiniLcmApiinto a new scopedCrdtSenseApi:GetSense×2,SubmitCreateSense,CreateSense,SubmitUpdateSense,UpdateSense×2,MoveSense,SubmitMoveSense,DeleteSenseThe sense link methods follow the precedent set by
AddPublicationandAddComplexFormType, which live in the service for the type being linked to:AddSemanticDomainToSenseandRemoveSemanticDomainFromSensemove toCrdtSemanticDomainsApiSetSensePartOfSpeechmoves toCrdtPartsOfSpeechApiCreateSenseChangesandVerifySenseBelongsToEntryare nowinternal staticonCrdtSenseApi, becauseCreateEntryand the example sentence methods (which move in later PRs) still use them.The method bodies are moved unchanged, with one exception.
CreateSenseused to check that the part of speech exists by calling the facade'sGetPartOfSpeech. It now runs anAnyAsyncquery against the repo directly, soCrdtSenseApidoesn't depend onCrdtPartsOfSpeechApi. The check and the exception it throws are the same.Test plan
LcmCrdt.Tests:SenseTests,PartOfSpeechTests,SemanticDomainTests,CreateEntryTests,ExampleSentenceTests,UpdateEntryTests,BasicApiTests,ConfigRegistrationTests(100 passed)FwLiteProjectSync.Tests: targeted sense move/reorder/create, semantic domain and part-of-speech sync and import tests (25 passed)🤖 Generated with Claude Code