-
Notifications
You must be signed in to change notification settings - Fork 23
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
[MER-2352] Make branch adoption work #121
Conversation
Current Aviator status
This PR was merged using Aviator. |
@@ -34,6 +34,24 @@ func Reparent( | |||
tx meta.WriteTx, | |||
opts ReparentOpts, | |||
) (*ReparentResult, error) { | |||
branchMeta, exist := tx.Branch(opts.Branch) | |||
if !exist { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this was not working previously if the current branch was not part of any existing stack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. That's why I added a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused by this -- can you hold off on merging for a second? I wanna look at it a bit more if that's okay!
8abc3af
to
5cb01ac
Compare
} | ||
branchMeta.Parent.Head = head | ||
} | ||
tx.SetBranch(branchMeta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to do this SetBranch
here.
I think it's effectively a no-op right now because we do the SetBranch
in reparentWriteMetadata
which is called on every code path (unless there's a conflict... is that an issue? 🙃).
At any rate, I think this is fine (as in it ~~works~~) but it's a little hard to follow.
5cb01ac
to
f516be7
Compare
Close #120.