ipfs / ipfs/kubo

kubo/client/rpc: drop/rework functional options

Open
#9,972 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
17.1k
Forks
3.2k
Avg merge
3d 18h
Merged PRs (30d)
11

Description

This is the developer journey for wanting to do something with the core API:

I want to list direct pins:

https://godoc.org/github.com/ipfs/interface-go-ipfs-core#PinAPI

Seems PinLs is the method I want. It takes some options. **What options?**

Ok, it takes a function which takes some settings: https://godoc.org/github.com/ipfs/interface-go-ipfs-core/options#PinLsOption (unhelpful docs: `PinLsOption pin ls option func `). But what I am supposed to be doing with this?

https://godoc.org/github.com/ipfs/interface-go-ipfs-core/options#PinLsSettings is not even documented. I could use that setting struct though, as it seems to have a way to set the Type, but how do I make it an option?

And we have a https://godoc.org/github.com/ipfs/interface-go-ipfs-core/options#PinLsOptions which returns the settings, also undocumented.

Scroll up and down, look for something useful, ctrl-F for "direct"... eventually after checking the code, realize there are these undocumented variables https://godoc.org/github.com/ipfs/interface-go-ipfs-core/options#pkg-variables .

There is a `Pin` object there of private pinOpts type. But that type casually implements all the things to make options useful, like the `Direct()` method I needed in the first place.

Suggestions (PinLs is an example, they apply to everything):

* If I could choose, I would rm -rf the options package and have a discussion on `PinLs(opts PinLsSettings)` vs `PinLs(type string)`. Removing functional spaguetti code and being way more explicit about what options an API method expects cannot be bad, even if it sometimes needs API breakage (currently we moved the API breakage from the methods to the options, which does not really solve anything when it needs to happen).
* Alternatively, I would expose public methods to build options. A PinLsOption should have constructors: PinLsDirect() etc... making it way easier to navigate and find those from the docs.
* PinLsSettings needs to be unexported as no one should be using this directly, given that there are options.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.