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

Sort map entries marshalling dag-cbor #204

Merged
merged 3 commits into from
Jul 27, 2021
Merged

Sort map entries marshalling dag-cbor #204

merged 3 commits into from
Jul 27, 2021

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jul 16, 2021

No description provided.

@rvagg rvagg requested review from warpfork and mvdan July 16, 2021 06:32
@mvdan
Copy link
Contributor

mvdan commented Jul 16, 2021

Nothing looks wrong here, though I think we should talk to Eric about codec map key sorting first, using the dag-json PR as a specific example. If and when we agree to merge that, then merging this should be easy :)

@warpfork
Copy link
Collaborator

I think we probably discussed things pretty sufficiently in #203 now --

Same one major change request before merge here: the cbor codec package actually (for better or for worse; worse, probably, for dependency sprawl, but nonetheless, for code DRYness) depends on the dagcbor package, so we should put a bool through here, so the cbor codec can continue to not sort.

@rvagg
Copy link
Member Author

rvagg commented Jul 19, 2021

If UnmarshalOptions and MarshalOptions are acceptable in #203 then I'll copy the approach here. (It'd also be possible to overload this even further to do more interesting things like having custom sorters, or "don't sort typednodes". But I'll restrain on feature creep.

@rvagg
Copy link
Member Author

rvagg commented Jul 21, 2021

k, I've added a MarshalOptions and UnmarshalOptions here with both control for links (encode and decode) and a MapSortMode for encode that lets you switch between sorting modes. Added a simple test for codec/cbor to exercise it like the others and it demonstrates lack of sorting (mind you ... there's zero definition of what a "cbor" IPLD codec should be encoding, it has some up before with users wanting to throw all sorts of tags into plain "cbor" blocks, we have no rules at all other than maybe a best-attempt at decoding what you're given).

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