feat!: add and connect missing context, remove RemovePinWithMode #23
Conversation
Now that PinWithMode() takes a context and return an error, there is not that much difference with Pin(). Those differences are: - Pin() store the root node and fetch the graph if recursive - Pin() accept an ipld.Node, PinWithMode() accept a CID (kinda meaningless) - Pin() accept a "recurse" boolean, PinWithMode() accept a Mode, but only Direct and Recursive values There is a case to be made to merge those or eliminate one.
|
Following discussion with @Jorropo, I made PinWithMode() return an error and in turn share most of the code with Pin() Now that PinWithMode() takes a context and return an error, there is not that much difference with Pin(). Those differences are:
There is a case to be made to merge those or eliminate one. |
|
As I merged Pin() and PinWithMode(), I noticed a difference in handling. This section was in Pin(), but absent in PinWithMode(). I have to say, I don't really understand what that does, and don't get what the comment wants to say. Maybe @gammazero could enlightens us? |
|
Another difference I missed (and changed): when trying to directly pin when a recursive pin already existed, Pin() would return an error while PinWithMode() would proceed and create a duplicated pin. I think the new behavior make sense: always return an error in that case. |
|
Corresponding PR in kubo: ipfs/kubo#9557 |
There was a problem hiding this comment.
Thanks for this! The code looks good. Once ipfs/kubo#9557 passes, I'll merge and release this.
hacdias
changed the title
Arguably,
RemovePinWithModecould be removed as it seems to be unused.