refactor: extract named backups list facade#294
refactor: extract named backups list facade#294ndycode wants to merge 1 commit intorefactor/pr3-storage-save-flagged-entryfrom
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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
getNamedBackupsfacade out oflib/storage.tson top of the latest storage leafValidation
npm run typechecknpm run lint -- lib/storage/named-backups-entry.ts lib/storage.ts test/named-backups-entry.test.tsnpm run test -- test/named-backups-entry.test.tsnote: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
this pr extracts the
getNamedBackupsfacade inlib/storage.tsinto a dedicatedlib/storage/named-backups-entry.tsmodule, following the same dep-injection pattern already used byrestoreAccountsFromBackupEntry. behaviour is unchanged; the extraction improves testability and keepsstorage.tsslimmer.lib/storage/named-backups-entry.ts— new thin entry that callscollectNamedBackupsvia fully injected deps (getStoragePath,collectNamedBackups,loadAccountsFromPath,logDebug)lib/storage.ts—getNamedBackupsis now a one-liner delegating togetNamedBackupsEntry; dep wiring is correcttest/named-backups-entry.test.ts— focused vitest case asserting all four deps are wired correctly and the return value propagates; adequate coverage for this pass-through layernamed-backups-entry.tsimportsNamedBackupSummaryfrom../storage.js(the parent facade) rather than from./named-backups.jswhere the interface is actually defined — recommend fixing to break the apparent type-level cycleConfidence Score: 5/5
restoreAccountsFromBackupEntryprecedent. the single test is sufficient for a pass-through facade. the only flag is theimport typesourced from the parent facade rather than the sibling definition — a p2 style issue that doesn't affect correctness or runtime behaviour.NamedBackupSummaryimport sourceImportant Files Changed
collectNamedBackupsvia injected deps — correct behaviour, but importsNamedBackupSummaryfrom../storage.jsinstead of the sibling./named-backups.jswhere it's defined, creating an apparent type-level circular referencegetNamedBackupsEntryand rewiresgetNamedBackupsto use it; wiring matches the same pattern asrestoreAccountsFromBackupEntry, all deps passed correctlySequence Diagram
sequenceDiagram participant caller as caller participant storage as lib/storage.ts<br/>getNamedBackups() participant entry as lib/storage/named-backups-entry.ts<br/>getNamedBackupsEntry() participant nb as lib/storage/named-backups.ts<br/>collectNamedBackups() caller->>storage: getNamedBackups() storage->>entry: getNamedBackupsEntry({ getStoragePath, collectNamedBackups, loadAccountsFromPath, logDebug }) entry->>entry: getStoragePath() → storagePath entry->>nb: collectNamedBackups(storagePath, { loadAccountsFromPath, logDebug }) nb-->>entry: NamedBackupSummary[] entry-->>storage: NamedBackupSummary[] storage-->>caller: NamedBackupSummary[]Prompt To Fix All With AI
Last reviewed commit: "refactor: extract na..."