Repository navigation
Document that LoadOrCreate() is a separate copy from Get() [patch] - #360
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
The CLAUDE.md sample put LoadOrCreate() and Get() side by side as if they were the same object. They are not: LoadOrCreate() reads the file into a new instance on every call, and the static QueueSave() and SaveIfRequired() act on Get() only. An edit made on a LoadOrCreate() instance and saved through the static helpers never reached disk. Making LoadOrCreate() return the singleton would stop it re-reading the file, which existing tests and consumers rely on, so this states the contract instead: on LoadOrCreate() and the static helpers in the XML docs, in the README, and in a corrected CLAUDE.md sample. Tests pin that the two are separate instances on the same file, and that each documented save path reaches disk. Fixes #330 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW2CxWzSsPmDZkw2R6J
|
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
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.
Fixes #330
What was wrong
The CLAUDE.md sample showed
LoadOrCreate()andGet()side by side as if they returned the same object. They don't.LoadOrCreate()reads the file into a new instance on every call, and the staticMySettings.QueueSave()/SaveIfRequired()act only on theGet()singleton. As a result, an edit made on aLoadOrCreate()instance and saved through the static helpers never reached disk.Choice of fix
The issue offers three options. This PR takes the documentation one, which its acceptance criteria allow ("or the docs clearly state it does not and the CLAUDE.md sample is corrected"). The two behavioural options would make the parameterless
LoadOrCreate()return, or seed, the singleton. That would stop it re-reading the file, and existing tests depend on the re-read: for example,TestLoadOrCreateDoesNotOverwriteExistingDatasaves throughGet()and then reads the file back withLoadOrCreate(). Downstream apps probably depend on it too. That change is a call for the maintainer, so it is left for a follow-up if you want it.Changes
LoadOrCreate()now say it returns a new instance, separate fromGet(), and that the instance must be saved through itself. The staticQueueSave()andSaveIfRequired()now say they act onGet()only.Tests
Three new
[DoNotParallelize]tests pin the documented contract:TestLoadOrCreateReturnsAnInstanceSeparateFromGet: the two are different instances that use the same file.TestEditOnGetSavedThroughStaticQueueSaveReachesDisk: an edit made throughGet()and saved with the static helpers reaches disk.TestEditOnLoadOrCreateSavedThroughItsOwnQueueSaveReachesDisk: an edit made throughLoadOrCreate()and saved with that instance's own helpers reaches disk.This change is documentation only, so no test can fail before it and pass after it. The tests pin current behaviour, so a later change to the contract has to update the docs too. Full suite: 123/123 passed locally on net10.0.
Note: the open PR #359 (CompatibilitySuppressions regeneration) may need to merge first if
main's pack step is still failing package validation.Generated by Claude Code