-
Notifications
You must be signed in to change notification settings - Fork 274
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
feat/refactor: add merge api #5259
Conversation
@@ -818,7 +820,7 @@ project/alice> merge /bob | |||
type Foo = Baz Nat Nat | Qux Text | |||
|
|||
-- project/bob | |||
type Foo = Baz Nat | BobQux Text | |||
type Foo = BobQux Text | Baz Nat |
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.
Any idea what caused these to flip?
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.
Not exactly, but I did investigate and they seem to have been wrong before. I manually walked through the transcript and observed that on project/bob
(in this example) the constructors do render in the new order. 🤷
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.
Updated explanation: in this PR we no longer reuse unique type UUID in merge, so any merge performed in a transcript can affect subsequent unique type constructor orders. We were going to circle back on that in a follow-up PR.
1fc2690
to
ed555a3
Compare
Overview
This PR refactors the merge API to make it more pure and suitable to call from Share.