Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Update to use new Builder interface for creating CIDs. #7

Merged
merged 3 commits into from
Aug 12, 2018

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Aug 10, 2018

No description provided.

@kevina
Copy link
Contributor Author

kevina commented Aug 11, 2018

This is separated into two commits. The first does the minimal amount of change to support the fact that a Merkeldag has a cid.Builder and not a cid.Prefix. The second renames methods and such so the name is consistent. If the second is too much I can drop it.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM but @schomatis may have opinions.

@schomatis for context, this patch allows us to pass around more complex cid "builders" instead of cid prefixes.

The key insight here is that cid.Prefix is informational, it tells you about an existing CID. However, we tend to use it as a "config" or CID builder. This patch replaces those uses with an actual CID builder (introduced here: ipfs/go-cid#53).

@schomatis
Copy link
Contributor

schomatis commented Aug 12, 2018

(@Stebalien Thanks for including me and for the context.) This is a great change! I agree that the old Prefix structure was being overextended.

@kevina I'm trying to build this PR but I'm having trouble finding QmWjvVuzD3Dqf3VznGAXUs1VqpG4dd35ounVCnLam8v7Xt (go-ipfs-posinfo), could you help me?

@kevina
Copy link
Contributor Author

kevina commented Aug 12, 2018

@schomatis Fixed. Forgot to pin that.

@schomatis
Copy link
Contributor

Thanks @kevina, this LGTM but I can't build against go-ipfs because of multiple errors, not related to the PR itself but to the current package management system, so I'm approving this.

(For background, go-cid is imported under the old and new versions, see gx deps dupes, which is more than understandable because it's one of the core packages in the IPFS stack, and gx-go rewrite when building the map ends up -by pure chance- pointing to the old version -that has overwritten the new version previously recorded on the map- and the result is that go-unixfs uses a dependency different from the one it declared in package.json. I have more to say about this but I'll discuss it in more appropriate issues, we need to fix this workflow or I fear that this is going to start blocking contributions.)

@kevina kevina merged commit 9900831 into master Aug 12, 2018
@ghost ghost removed the status/in-progress In progress label Aug 12, 2018
@kevina
Copy link
Contributor Author

kevina commented Aug 12, 2018

@schomatis thanks, I am not 100% following you, but I mostly agree. The way to get around this is to use gx-go link.

In any case this is an API change so to build so it won't build anyway. I will update ipfs/kubo#5281 and id necessary split the automatic inline part into a separate P.R. to avoid blocking anything.

@Stebalien Stebalien deleted the kevina/builder branch August 12, 2018 21:09
@schomatis schomatis mentioned this pull request Aug 13, 2018
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 25, 2023
Update to use new Builder interface for creating CIDs.

This commit was moved from ipfs/go-unixfs@9900831
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants