Skip to content

feat: testutil.RandomCID() is a cidlink.Link#72

Draft
Peeja wants to merge 2 commits intomainfrom
feat/randomcid-is-cidlink
Draft

feat: testutil.RandomCID() is a cidlink.Link#72
Peeja wants to merge 2 commits intomainfrom
feat/randomcid-is-cidlink

Conversation

@Peeja
Copy link
Member

@Peeja Peeja commented Nov 13, 2025

This has always been a link and not an actual CID. Fixing the naming would mean a whole bunch of test code changes for little value, but we can at least make it return cidlink.Link explicitly and not ipld.Link generally, which means that test code no longer has to type-assert it to cidlink.Link to get the cid.CID from inside it.

This has always been a link and not an actual CID. Fixing the naming
would mean a whole bunch of test code changes for little value, but we
can at least make it return `cidlink.Link` explicitly and not
`ipld.Link` generally, which means that test code no longer has to
type-assert it to `cidlink.Link` to get the `cid.CID` from inside it.
}

func RandomCID(t *testing.T) datamodel.Link {
func RandomCID(t *testing.T) cidlink.Link {
Copy link
Member

Choose a reason for hiding this comment

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

I know this would be breaking, but I really wish the method returned a cid.Cid, instead of whatever a cidlink.Link is..

Copy link
Member Author

Choose a reason for hiding this comment

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

I know. cidlink.Link is just a link that's implemented with a CID. It's weird because I don't know of any other kinds of links in IPLD? It seems like they have to be CIDs? But technically ipld.Link is a generic interface, and cidlink.Link implements that interface using CIDs, so you have to know you have one to pull the actual CID out.

Ideally, this would be called RandomLink() and RandomCID() would return a cidCid, but…well, someday, I hope.

Copy link
Member

@alanshaw alanshaw Nov 14, 2025

Choose a reason for hiding this comment

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

I know this would be breaking, but I really wish the method returned a cid.Cid, instead of whatever a cidlink.Link is..

I mean, at least you can get the cid.Cid out of this now without type asserting...

I feel like this might be my fault...sorry. I would get behind a RandomCID -> RandomLink rename and a new RandomCID method that actually returns a cid.Cid.

@Peeja
Copy link
Member Author

Peeja commented Nov 13, 2025

Hmm, turns out it's not actually a completely non-breaking change. 😕

Maybe it's worth renaming it after all. I'm going to put this on ice until we're a little less busy.

@Peeja Peeja marked this pull request as draft December 3, 2025 17:17
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