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

Write is_supported_content_addressed_uri #152

Merged
merged 2 commits into from
Apr 25, 2019

Conversation

njgheorghita
Copy link
Contributor

What was wrong?

Already used this function in both ethpm-cli and the ipfs pinner, so it seems like it belongs here.

Cute Animal Picture

image

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

I'm 👍 on this as-is for now but this will very likely need to be extensible at some point to allow 3rd party URIs to be registered and accepted. Probably a similar mechanism to the eth-abi library with some kind of Registry implementation that allows new ones to be added with the library supporting the common use case with a default registry with the built-in URIs pre-registered.

@njgheorghita
Copy link
Contributor Author

@pipermerriam What exactly do you have in mind by '3rd party URIs'? Can you give an example? I was under the impression that it was pretty much IPFS, Github blob, Swarm (eventually) that will be supported/valid content-addressed uris.

@pipermerriam
Copy link
Member

@njgheorghita bitbucket, gitlab, gists, .... ipfs-gateway uris (there can be more than one gateway, cloudflair has one). The EthPM spec is intentionally open about the definition of a content addressable URI and thus, there are tons of options out there for people to include the content hash in different URI schemes.

@njgheorghita njgheorghita merged commit 727ca56 into ethpm:master Apr 25, 2019
@njgheorghita njgheorghita deleted the is-supported-uri branch April 25, 2019 11:14
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.

2 participants