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

Deserialize for array groups with optional fields #204

Merged
merged 2 commits into from
Aug 9, 2023

Conversation

rooooooooob
Copy link
Collaborator

Fixes #203

Fixes #154 as this issue popped up while making sure optional fields were very well supported.

@rooooooooob rooooooooob added the bug Something isn't working label Jul 27, 2023
@rooooooooob
Copy link
Collaborator Author

rooooooooob commented Jul 27, 2023

TODO: add test for preserve-encodings=true

Edit: Done

@rooooooooob
Copy link
Collaborator Author

This will also be useful for when we generate conway.cddl. And any time regenerating things from alonzo/babbage without needing to hand-edit the optional field for the tx outputs.

@SebastienGllmt
Copy link
Collaborator

Can we be sure to update the docs if relevant when we add new features?

Copy link
Contributor

@gostkin gostkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like one test is failing:

test test::rust_wasm_split ... FAILED

@rooooooooob
Copy link
Collaborator Author

@gostkin the test is failing on master it seems too. I fixed it in #207

@rooooooooob
Copy link
Collaborator Author

@SebastienGllmt I don't think the docs need changing, at least in its current format. We still don't support deserialization for 100% of types, although it is much, much better now, so I think the current statement of * Deserialization for almost all supported types is fine. Only more ambiguous array types aren't supported now so most real-world cddls written to have parsing logic not a pain to write should be fine. I thought we are using issues for unsupported things not the docs?

Fixes #203

Fixes #154 as this issue popped up while making sure optional fields
were very well supported.
@rooooooooob rooooooooob merged commit d6086cf into master Aug 9, 2023
1 check passed
@rooooooooob rooooooooob deleted the array-opt-field-deser branch August 9, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Needs rebase
Projects
3 participants