Repository navigation
Refactor: one endMoving() for the move and the delete from the Device (#890) - #948
Merged
Merged
Conversation
…#890) moveDriveOnlyTakes and releaseFromDevice each had the same block that clears isMoving and resumes the edits that waited for a move. It is now endMoving(), and releaseFromDevice lets the Drive go with a defer (once, on every way out). One difference: the Drive stays held through the few lines that update the moves list and read the Drive's Take.json, where it was let go before them. Assisted-by: Claude Code:claude-sonnet-5-5
Skeyelab
enabled auto-merge (squash)
October 23, 2026 03:51
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
refactor: merge after the v1.1.16 tag. It touches the move and delete code, so Eric approves it after the tag.
One item of #890.
moveDriveOnlyTakesandreleaseFromDeviceeach carried the same block that clearsisMovingand resumes the edits waiting on a move. It is nowendMoving().releaseFromDevicealso lets the Drive go with adefer, so it is let go once on every way out and none of its early returns needs its ownaccess.end().No red test is possible: it changes no behavior that a test can see. One small difference, found by the Code Reviewer:
releaseFromDevicenow holds the Drive until it returns, through the few lines that update the moves list, regenerate the report and read the Drive'sTake.json(Self.readTake), where before it was let go right after the delete ran. That is safer (the read is on the Drive), and the access still ends beforeendMovinglets waiting edits go (the order is commented). The state changes in the move path keep their order apart fromfoundTakesToMoveandisMovingswapping places inside one synchronous defer.Added
driveLetGoOnceinRecorderReleaseTests: a countingstop:closure shows the Drive is let go exactly once for a kept and a released delete, andisMovingis false afterward. It passes before and after the refactor (a guard, not a red test). The existingMoveTriggerTests,MoveStandInTestsandRecorderReleaseTestscover both functions and the waiters.VERIFY_QUICK=1 ./scripts/verify.shpasses.