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

Consider moving finish() on Serialize #19

Closed
mitsuhiko opened this issue Feb 10, 2022 · 1 comment
Closed

Consider moving finish() on Serialize #19

mitsuhiko opened this issue Feb 10, 2022 · 1 comment
Labels
design Design considerations

Comments

@mitsuhiko
Copy link
Owner

mitsuhiko commented Feb 10, 2022

It seems a bit odd that the main purpose of finish on Serialize is to undo state changes for the nested emitters, but it can't be conditional on the state of the emitters. If the method were to move onto the emitters then the state of the emitter can be used to undo the state in the serializer.

Relatedly recent changes now call finish for atoms as well on the deserializing sink. This seems wasteful.

@mitsuhiko mitsuhiko added the design Design considerations label Feb 10, 2022
@mitsuhiko
Copy link
Owner Author

I tried a few things now and while it's odd that you can't access the the emitters that's not much of an issue as the main use of finish is to wrap the whole thing anyways. I will close this now as moving this to the emitters actually makes the main use of it a lot less ergonomical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design considerations
Projects
None yet
Development

No branches or pull requests

1 participant