feat(files.add): update files.add API #56
Conversation
daviddias
force-pushed
the
feat/upgrade-files.add
branch
from
e2a3246
to
f385030
Compare
|
LGTM! |
| @@ -28,7 +28,8 @@ If no `content` is passed, then the path is treated as an empty directory | |||
| ```js | |||
| { | |||
| path: '/tmp/myfile.txt', | |||
| node: DAGNode | |||
| hash: 'QmHash', // base58 encoded multihash | |||
There was a problem hiding this comment.
This should not be a base58 encoded multihash. Looking at the future we are introducing cid + ipld soon. That means we have to break this interface at that point to a degree. This should be mulithash buffer/object something like that.
There was a problem hiding this comment.
Thank you, I was thinking of that too, specially with the introduction of CID.
However, there will be a CID string version anyway, so the question emerges, do we have a raw multihash/CID or the string version
There was a problem hiding this comment.
CIDs will allow for different versions of encoding, i.e. non base58. This interface though binds itself to using base58 which I find the most concerning part.
There was a problem hiding this comment.
got it. Right now, the content Ids are presented as base58 Encoded multihash, because there is no multibase nor multicodec. Once we have CID, the description will change to 'String representation of the CID', cause it is a self described Id :D
daviddias
force-pushed
the
feat/upgrade-files.add
branch
from
f385030
to
dbae1c0
Compare
|
I'm finishing up the changes in js-ipfs-api. Please review these meanwhile if you can. |
daviddias
force-pushed
the
feat/upgrade-files.add
branch
from
dbae1c0
to
0f1c6e8
Compare
|
Done both, review please :) ping @dignifiedquire @haadcode @noffle @nginnever @RichardLitt @victorbjelkholm @shamb0t @xicombd |
| @@ -2,7 +2,7 @@ | |||
| "name": "interface-ipfs-core", | |||
| "version": "0.7.2", | |||
| "description": "A test suite and interface you can use to implement a IPFS core interface.", | |||
| "main": "lib/index.js", | |||
| "main": "src/index.js", | |||
There was a problem hiding this comment.
change before merge :P
|
except the two things above all lgtm |
daviddias
force-pushed
the
feat/upgrade-files.add
branch
from
0f1c6e8
to
dfa0094
Compare
|
Thank you for the CR @dignifiedquire :):) |
@dignifiedquire @haadcode @victorbjelkholm @noffle @shamb0t @xicombd @JGAntunes @nginnever would like your check/:+1: on this :)
ref: #29