Skip to content
This repository was archived by the owner on Aug 15, 2021. It is now read-only.

Fixups for conversions #200

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

npmccallum
Copy link

  1. Add tests to ensure From conversions and serializations are consistent.
  2. Fix the inconsistencies.
  3. New conversions for increased ergonomics.

Serde uses a blanket impl for serialization. This change fixes the From
conversions to match serde's behavior. Without this, the user of serde_cbor
could run into awkward bugs where values of the same type get serialized
different ways; all hidden behind an innocent looking .into().
It made sense to avoid these in the past because they caused allocation.
However, in order to fix conversion mismatches with serde, we had to adopt
allocation. Since we have now accepted allocation, it now makes sense to
add these conversions for ergonomics.
Copy link
Contributor

@stevenroose stevenroose left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

2 participants