Repository navigation
refactor: drop the round-1 review launch-marker migration - #159
Merged
Merged
Conversation
Round-1 markers are date-keyed files at most 14 days old; the reap deletes them anyway. Removes the migration block and its four tests.
Merged
3 of 6 tasks
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.
Summary
Removes the migration of round-1
review-launched-$DATEmarkers frombin/review.sh, and the four tests that pinned it. Those markers were date-keyed files that the 14-day reap deletes anyway, so the code only ever handled installs from before the content-digest marker existed.One reviewable idea. This is the part of the legacy-path cleanup (#158 follow-up) that held up; the L2 argv fallback and the inline session-enumeration fallback stay (see below).
Worst case after this change: a user who ran the round-1 code earlier the same day they upgrade gets one duplicate workspace popup on that day. It fails open, nothing is lost, and the reap clears the leftover file within 14 days. The migration and the digest scheme landed together in
3fbad69, so very few installs can hold a round-1 marker.Status
tests/review-cmux.shmainin this sandbox.tests/review-skip.shtests/pending-triage.shreview.sh+review-cmux.shtests/run-all.shrun.shpart, which is not in this PRa5d700d. It found no leftover reference to the migration, no orphaned test helper, and nothing else that globsreview-launched-*. Its one nit (a historical comment atreview.sh:312) is left alone./debate:run tightNot in this PR
bin/run.sh. About 20 tests intests/run-all.shsimulate installs with a partial or oldadapters/tree, which is what those fallbacks serve. Removing them needs those tests rewritten first.Test plan
bash tests/review-cmux.sh,bash tests/review-skip.sh,bash tests/pending-triage.shtests/run-all.shon the merged result