Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

Let ID implement encoding.Binary[Un]Marshaler and encoding.Text[Un]Marshaler #44

Merged
merged 2 commits into from
Feb 25, 2019

Conversation

hsanjuan
Copy link
Contributor

When encoding a map[peer.ID]something to JSON it cannot be parsed back because ID doesn't implement TextMarshaler.

While the BinaryMarshaler is not strictly necessary, it is a safeguard in case at some point an ID is no longer an alias for a string.

@hsanjuan hsanjuan requested a review from raulk February 25, 2019 19:35
@ghost ghost assigned hsanjuan Feb 25, 2019
@ghost ghost added the in progress label Feb 25, 2019
@hsanjuan hsanjuan requested a review from Kubuxu February 25, 2019 19:36
@hsanjuan
Copy link
Contributor Author

Thank you @vyzo

@hsanjuan hsanjuan merged commit 9b0c59c into master Feb 25, 2019
@hsanjuan hsanjuan deleted the marshalers branch February 25, 2019 22:54
@raulk
Copy link
Member

raulk commented Feb 25, 2019

@vyzo always front-running me 😛

@hsanjuan
Copy link
Contributor Author

@raulk should I have waited? I think in IPFS-land merging after approval is ok but I'm not sure if libp2p is stricter here.

@raulk
Copy link
Member

raulk commented Feb 26, 2019

@hsanjuan No, it’s fine 👍 Unless it’s something more controversial that requires more buy-in from the team; this wasn’t the case.

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

Successfully merging this pull request may close these issues.

3 participants