Remove dead segment-pruning workarounds from collect_stack_segments#13745
Draft
mtsgrd wants to merge 1 commit into
Draft
Remove dead segment-pruning workarounds from collect_stack_segments#13745mtsgrd wants to merge 1 commit into
mtsgrd wants to merge 1 commit into
Conversation
Remove five TODOs and ~30 lines of workaround code that pruned remote-only segments from the front, back, and tail of collected stacks. Also remove the !is_empty() guard from is_pruned that special-cased empty segments. These workarounds were dead code: the graph builder always places remote refs (e.g. origin/A) on their own graph nodes, separate from the first-parent path. When commits lack a local ref, their segment becomes anonymous (ref_info: None), which is_entrypoint_or_local already treats as local via is_none_or. So remote-only segments never appeared in positions these workarounds would have caught. Four new test scenarios confirm this: - entrypoint_on_workspace_commit: tag on workspace commit as entrypoint - remote_only_stack_top: local branch deleted, only origin/A remains - remote_trailing_local_stack: local B stacked on remote-only origin/A - remote_ref_as_stack_top: workspace merges a never-local remote ref All four produce correct graph/workspace output without the old pruning, verifying the workarounds were unnecessary.
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.
Remove five TODOs and ~30 lines of workaround code that pruned
remote-only segments from the front, back, and tail of collected
stacks. Also remove the !is_empty() guard from is_pruned that
special-cased empty segments.
These workarounds were dead code: the graph builder always places
remote refs (e.g. origin/A) on their own graph nodes, separate from
the first-parent path. When commits lack a local ref, their segment
becomes anonymous (ref_info: None), which is_entrypoint_or_local
already treats as local via is_none_or. So remote-only segments
never appeared in positions these workarounds would have caught.
Four new test scenarios confirm this:
All four produce correct graph/workspace output without the old
pruning, verifying the workarounds were unnecessary.