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

Export protocol codes #11

Merged
merged 2 commits into from
Feb 27, 2019
Merged

Export protocol codes #11

merged 2 commits into from
Feb 27, 2019

Conversation

backkem
Copy link
Contributor

@backkem backkem commented Dec 20, 2018

Export the protocol codes so they can be used in whyrusleeping/mafmt.
I also added the 'single source of truth' comment as in go-multiaddr. I could not find a decision in this regard. Let me know if there is a single source of truth already, I can change the PR to reflect it.

dns.go Outdated Show resolved Hide resolved
@hsanjuan
Copy link
Contributor

This codes are already exported as Dns4Protocol.Code so this should not be necessary?

@backkem
Copy link
Contributor Author

backkem commented Dec 20, 2018

Good point. It does break the 'uniformity' of whyrusleeping/mafmt thought.
Regarding decentralization: Makes sense. I was just trying to mirror the JavaScript implementation.
Feel free to close if you think I should use XxxProtocol.Code instead.

@hsanjuan
Copy link
Contributor

I don't think it hurts (it's a mild improvement), but you will have to Gx-bubble more things.

@ghost ghost assigned raulk Feb 27, 2019
@ghost ghost added the status/in-progress In progress label Feb 27, 2019
@raulk raulk merged commit f755be0 into multiformats:master Feb 27, 2019
@ghost ghost removed the status/in-progress In progress label Feb 27, 2019
@raulk
Copy link
Member

raulk commented Feb 27, 2019

@backkem thanks for the contribution! 💪

@backkem backkem deleted the export-codes branch February 27, 2019 12:20
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