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

Unify deser impl (redux) #488

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Conversation

ijackson
Copy link
Contributor

This is #338 reworked to apply to current main, and with the test case simplified as requested.

This tests, in particular, that the error messdage is as expected
including the right key and filename.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
This had open-coded copies of the impl for Value.  Replace them with
calls to the impl for Value.  This reduces duplication.  It would
allow us to change the impl for Value.

Use a macro for the many very-similar functions.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
@ijackson ijackson mentioned this pull request Oct 23, 2023
@matthiasbeyer matthiasbeyer merged commit 5ff62b7 into rust-cli:master Oct 23, 2023
15 checks passed
@polarathene
Copy link
Collaborator

I haven't checked yet, but heads-up prior to all the rework done recently to the ser.rs and de.rs, I had identified the cause of #461 (comment) and fix. Although that's less obvious now.

Might be worth verifying if you've implicitly resolved that issue too, or added more drift 😅

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

Successfully merging this pull request may close these issues.

3 participants