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

A serialization error we were seeing in repl usage #1376

Merged
merged 2 commits into from
Jul 21, 2015
Merged

Conversation

ianoc
Copy link
Collaborator

@ianoc ianoc commented Jul 20, 2015

Test that should fail followed by fix

Two things:

we only expect to use toPipe internally, marking it package private helps with where it gets used.

Also the error stems from merging of flowdefs which need to copy the Boxed info between them
@johnynek
Copy link
Collaborator

I don't see why this fixes it. Why isn't calling toPipe their okay?

johnynek added a commit that referenced this pull request Jul 21, 2015
A serialization error we were seeing in repl usage
@johnynek johnynek merged commit 4b4bcf8 into develop Jul 21, 2015
@johnynek johnynek deleted the ianoc/fixBug branch July 21, 2015 00:47
@ianoc
Copy link
Collaborator Author

ianoc commented Jul 21, 2015

toPipe there should be fine, it just seemed wrong since we'd be doing the operations twice. though not likely to produce an incorrect result

The fix was the other case, there is a few paths from which we merge flowdef's, and i guess our updater stuff was missing those.

@johnynek johnynek added the bug label Jul 21, 2015
@ianoc ianoc mentioned this pull request Aug 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants