Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Implement isLink() #126

Closed
vmx opened this issue Apr 27, 2018 · 6 comments
Closed

Implement isLink() #126

vmx opened this issue Apr 27, 2018 · 6 comments
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue status/ready Ready to be worked

Comments

@vmx
Copy link
Member

vmx commented Apr 27, 2018

The isLink() function was removed from the IPLD Format spec (ipld/interface-ipld-format#23), in favour of putting this functionality into js-ipld instead.

The implementation from js-ipld-dag-pb can be used.

@vmx vmx added help wanted Seeking public contribution on this issue exp/novice Someone with a little familiarity can pick up status/ready Ready to be worked labels Apr 27, 2018
@jonahweissman
Copy link
Contributor

I'd like to tackle this, if that's okay.

@jonahweissman
Copy link
Contributor

So, it looks like isLink(), as implemented in js-ipld-dag-pb, accepts a binary blob, and then uses resolve() to find the link corresponding to the given path. However, in js-ipld, the function equivalent to resolve() seems to be get(), which accepts a CID. Would it be better to try to determine the CID from the binary blob, or just change isLink() to take a CID as an argument?

It's also quite possible there's something I'm missing. I'm still trying to get a sense of how all these pieces fit together.

@jonahweissman
Copy link
Contributor

To clarify my question a bit: if isLink() operates on binaryblobs, how do we know which resolver to use?

@vmx
Copy link
Member Author

vmx commented Aug 20, 2018

The more I think about it, the more i wonder if isLink() is actually a useful thing. What is its use case?

Things that came to my mind, but where there are better solutions:

  • You don't know the structure of the node and you want to explore it: In this case I'd expect that you want the other contents as well, so you'd call (on the format level) resolve('/').
  • You want to traverse the DAG: Then you probably know the structure of the node and you don't want to traverse any link, but a specific one. So you know that it is a a link and don't need isLink().

@jonahweissman @olizilla @mikeal @diasdavid @Stebalien Can you think of cases where you'd want to have an isLink() function?

@mikeal
Copy link

mikeal commented Aug 20, 2018

I can't think of a great case because the path resolver stops at a link and links are easy to identify as the return value, especially if we move to using CID instances.

However, I could definitely see a use for either a links method that returned all links and/or a tree option that noted which paths were links.

@vmx
Copy link
Member Author

vmx commented Sep 11, 2018

I'm closing this issue as "won't fix". Having a isLink() method is not really useful.

@jonahweissman Thanks for looking into this and getting the discussion started.

@vmx vmx closed this as completed Sep 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

3 participants