add Pin.IsPinned(..)

[MichaelMure]

This PR add the following function to the Pin API:

// IsPinned returns whether or not the given cid is pinned
// and an explanation of why its pinned
IsPinned(context.Context, path.Path, ...options.PinIsPinnedOption) (string, bool, error)

This is needed to ultimately implement ipfs pin ls <cid>.

To do so, it was also needed to migrate the PinLsOptions into their own namespace.

Note: to avoid conflicts, this PR is chained with #49 so the diff here will include both until #49 is merged.

[MichaelMure]

cc @aschmahmann @Stebalien

[MichaelMure]

Rebased on master and fixed go fmt complaining.

[hsanjuan]

hey @MichaelMure, thanks for this. I left some comments. This library has many dozens of golint warnings and it would be nice if the files touched by the PR did not show any more issues. Since this is the core interface, it is specially important to have amazing developer docs here.

[MichaelMure]

@hsanjuan I'm torn on this one, I suppose mainly due to a question of taste. At the moment in the other namespace (block, object ...), all the options are merged at the root: Object.Type() is an option for Object.New, Object.Datatype is an option for Object.Put ... It works as long as there is no conflict, but get messy as soon as there is one. See Pin.Recursive and Pin.RmRecursive.

Adding IsPinned add 5 more conflicts in there and I suppose more conflict will come up in the future. That's why I created a sub-namespace for Ls and IsPinned.

I know it's a hard change but shouldn't that problem be addressed now rather than later ? Maybe instead of having options exposed as Pin.Ls.XX it should be PinLs.XX to remove a level ?

WDYT ?

Maybe @Stebalien would like to step in here ?

[MichaelMure]

Otherwise, I agree about the documentation.

[hsanjuan]

Adding IsPinned add 5 more conflicts in there and I suppose more conflict will come up in the future. That's why I created a sub-namespace for Ls and IsPinned.

I know it's a hard change but shouldn't that problem be addressed now rather than later ? Maybe instead of having options exposed as Pin.Ls.XX it should be PinLs.XX to remove a level ?

I'm a bit lost. I think it's ok having two options (PinLsSettings and PinIsPinnedSettings) but they could use the the same embedded type (until there is a reason to separate them or customize behaviour).

type PinLsSettings struct {
  typeSettings
}

etc. in the case of object, Type and Datatype are different things (one is for empty,unixfs-dir etc, the other is for base64,text). But here Type and WithType represent the same thing (recursive, or direct)...

[MichaelMure]

@hsanjuan Sorry to ask you this but could you take over this one? I'll probably be unavailable for a week+.

[hsanjuan]

@hsanjuan Sorry to ask you this but could you take over this one? I'll probably be unavailable for a week+.

I don't mind taking over, but my availability is not much better than yours :/

[MichaelMure]

@hsanjuan I mean if you could shuffle the code around the way you want it, I can deal with the ground work around (doc and updating ipfs/kubo#6774)

[hsanjuan]

ok, I have come to a better understanding here. Duplicating code is the only way I see of not making the core API option parsing even more convoluted than it already is.

@Stebalien LGTM.

[hsanjuan]

@MichaelMure but do fix the docstrings please...

[MichaelMure]

@hsanjuan thanks for the help :)