-
Notifications
You must be signed in to change notification settings - Fork 58
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
Write Builders manually #101
Conversation
A lot of noise from cargo fmt (I assume). Could you split it up? |
I don't think the rustfmt noise is too present. Do you mean we should rustfmt before this PR? |
No, I mean one commit for rustfmt and one with the builder changes. |
I am not able to split them. |
Same thing. ;) |
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.
Lgtm
I'd like to see some tests though. Too prevent the previous situation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
106: serde skip serialization r=therealprof a=burrbull Part of #101 This change should not be breaking. Co-authored-by: Andrey Zgarbul <[email protected]>
276845c
to
1b5234d
Compare
@Emilgardis What is the status of this PR? I'm trying to remove an outdated in-tree version of |
Closed. See #115 |
115: Write Builders manually 2 r=Emilgardis a=burrbull #101 without changes in Error hierarchy. Should slightly be easier to review. Co-authored-by: Andrey Zgarbul <[email protected]>
No description provided.