Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repo sync: don't propagate downstack history for unsubmitted branches #527

Merged
merged 2 commits into from
Dec 21, 2024

Conversation

abhinav
Copy link
Owner

@abhinav abhinav commented Dec 21, 2024

Add a test for how merge history propagation behaves
for upstack branches that are unsubmitted.

Suppose I have feat1 -> feat2:

  • feat1 is ready and I submit a CR
  • feat2 is not yet ready for review, so it is unsubmitted
  • while I'm still working on feat2, feat1 gets approved and I merge it
  • I then submit feat2

feat2 still shows feat1 in its downstack history.

We can change this behavior in the future if it makes sense
but right now I lean towards more information over less.

@abhinav
Copy link
Owner Author

abhinav commented Dec 21, 2024

When bubbling up downstack history,
don't nuke the history of a merged branch until it's deleted.
If this branch fails to be deleted/untracked for any reason,
we'll still have this history to work with.
Add a test for how merge history propagation behaves
for upstack branches that are unsubmitted.

Suppose I have `feat1 -> feat2`:

- feat1 is ready and I submit a CR
- feat2 is not yet ready for review, so it is unsubmitted
- while I'm still working on feat2, feat1 gets approved and I merge it
- I then submit feat2

Right now--by accident--it works so that feat2 receives the history:
it will show feat1 as its merged downstack even though feat1 was merged
by the time feat2 was restacked on main and submitted.

My intuition is that this should NOT be supported,
but it's also coming mostly for "free".

I'm opening up this PR and test case to discuss this behavior
with anyone who cares.
@abhinav abhinav force-pushed the repo-sync-dont-nuke-merged-history branch from 8f3612b to 4ff4b61 Compare December 21, 2024 18:29
@abhinav abhinav force-pushed the nav-history-unsubmitted-test branch from 27e1114 to 2e3cdde Compare December 21, 2024 18:29
@abhinav abhinav added the skip changelog PRs that don't need a changelog. label Dec 21, 2024
Base automatically changed from repo-sync-dont-nuke-merged-history to main December 21, 2024 18:37
@abhinav abhinav changed the title test(repo sync): navigation history for unsubmitted branches repo sync: don't propagate downstack history for unsubmitted branches Dec 21, 2024
@abhinav abhinav merged commit 5dfd350 into main Dec 21, 2024
10 of 11 checks passed
@abhinav abhinav deleted the nav-history-unsubmitted-test branch December 21, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changelog PRs that don't need a changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant