Skip to content

Sort map entries marshalling dag-cbor#204

Merged
rvagg merged 3 commits intomasterfrom
rvagg/dagcborsort
Jul 27, 2021
Merged

Sort map entries marshalling dag-cbor#204
rvagg merged 3 commits intomasterfrom
rvagg/dagcborsort

Conversation

@rvagg
Copy link
Copy Markdown
Member

@rvagg rvagg commented Jul 16, 2021

No description provided.

@rvagg rvagg requested review from mvdan and warpfork July 16, 2021 06:32
@mvdan
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 rvagg force-pushed the rvagg/dagcborsort branch from 20a0caf to 9250c74 Compare July 21, 2021 02:48
@rvagg rvagg force-pushed the rvagg/dagcborsort branch from 9250c74 to fa60a84 Compare July 21, 2021 02:55
@rvagg
Copy link
Copy Markdown
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