-
Notifications
You must be signed in to change notification settings - Fork 566
fix: serde and packable on empty #18584
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
Conversation
5800704 to
03f2d49
Compare
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
mverzilli
left a comment
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.
Always a pleasure to review a one file PR :D
Only comment is we seem to be checking multiple times the same cases (params>1, params==1, params==0) so it would be nice to reduce that duplication, but I'm not sure whether that's possible in Noir and more specifically in Noir macros
Yes, there is a large overlap between serde and packable. I think we had a plan to eventually merge the Serialize and Deserialize traits into one trait and at that point they would become even more similar. So would wait until we do that to bother with this. |
03f2d49 to
42f1ead
Compare
Fixes #17586 Forgot to handle the empty struct case when implementing the derivation functionality. This PR fixes it.
42f1ead to
b4d0079
Compare

Fixes #17586
Forgot to handle the empty struct case when implementing the derivation functionality. This PR fixes it.