chore(api): explain the three unwired app-settings routes - #114
Merged
Merged
Conversation
#90 established that every APIClient function without a call site must say why it is kept, and #113's review sweep found three that don't: deleteAppSettings, appDeviceSettings and putAppDeviceSettings. All three are deliberate, not oversights — the per-device document is unwritten because iOS has no phone-specific persisted state worth syncing yet, and deleting the account document isn't an action any surface offers. Recording the reasoning so the next sweep doesn't re-litigate it, or delete routes that exist to give the first per-device setting an obvious home. No behaviour change; comments only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017bss5MgZa7Jvj2m9zdaUd1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the loose end #113's review sweep reported. #90 established the rule that every
APIClientfunction with no call site must carry a one-line comment saying who calls it or why it iskept;
deleteAppSettings,appDeviceSettingsandputAppDeviceSettings(added by #89) had neithera caller nor an explanation.
All three are deliberate rather than oversights, so the fix is to record the reasoning — not to
delete them and not to wire them.
What's included
deleteAppSettings— no caller. The account document is the sync service's own state, notsomething a user asks the app to discard; the web's Applications section forgets a device, which
is
forgetAppDevice(deviceId:). Kept so the six-route contract this file models stays whole.appDeviceSettings/putAppDeviceSettings— no callers. iOS writes only the account-leveldocument, because it has no genuinely phone-specific persisted state worth syncing yet (the
account/device split recorded in feat(settings): adopt the cross-device app-settings sync service #89). They exist so the first such setting has an obvious home
rather than being appended to the shared document by default — which is the mistake that would be
hard to undo later.
Comments only. No behaviour change.
Testing
xcodebuild … -parallel-testing-enabled NO -skip-testing:InterlinedListTests/E2EReadOnlyTests test— 1335 tests, 0 failures.
/ios-reviewnow returns no unexplained entries.🤖 Generated with Claude Code