complete files.stat with the 'with-local' option #227
Conversation
SPEC/FILES.md
Outdated
| @@ -618,6 +618,7 @@ Where: | |||
| - `options` is an optional Object that might contain the following keys: | |||
| - `hash` is a Boolean value to return only the hash. | |||
| - `size` is a Boolean value to return only the size. | |||
| - `with-local` is a Boolean value to compute the amount of the dag that is local, and if possible the total size. | |||
There was a problem hiding this comment.
When options have -, we translate them to camel case. withLocal
There was a problem hiding this comment.
Will do, but it's currently not respected in js-ipfs-api. For instance: https://github.com/ipfs/js-ipfs-api/blob/master/test/files.spec.js#L93
There was a problem hiding this comment.
Seems that one passed by review. That said, we still want to keep the camelCase version and support the dash for backwards compatibility
There was a problem hiding this comment.
I see that lodash is used exactly once in js-ipfs-api. Nothing against using opts = _.mapKeys(opts, (v, k) => _.kebabCase(k)) ?
That would support both version and some more easily.
js/src/files-mfs.js
Outdated
| it('stat withLocal file', (done) => { | ||
| if (!withGo) { | ||
| console.log('Not supported in js-ipfs yet') | ||
| return done() |
There was a problem hiding this comment.
call this.skip(). You will have to replace the arrow function by regular function
There was a problem hiding this comment.
This is the way it's done in all the codebase. Do you prefer to change that here or in a single pass in another PR ?
There was a problem hiding this comment.
can you change the others first in a PR and update this one to match? thank you :)
js/src/files-mfs.js
Outdated
| if (!withGo) { | ||
| console.log('Not supported in js-ipfs yet') | ||
| return done() | ||
| } |
js/src/files.js
Outdated
| it('stat outside of mfs', () => { | ||
| if (!withGo) { | ||
| console.log('Not supported in js-ipfs yet') | ||
| return done() |
|
@diasdavid I changed for |
|
This PR broke most of the @MichaelMure Can you please have a look? |
|
@vmx you will need ipfs-inactive/js-ipfs-http-client#695. Wanna update deps there? This is one of the reasons why I asked you for a issue that lists all the PR to make js-ipfs-api tests. There are so many PR's in flight that is hard to manage. |
|
@vmx Thank for taking care of that. |
|
@MichaelMure It wasn't me, @diasdavid fixed those tests. |
|
Errr, thanks @diasdavid ^^ |
Following ipfs/kubo#4638, this new option is present in go-ipfs that enable to know if and how much of a dag is present locally.
Now, in the go-ipfs's http API,
WithLocality,LocalandSizeLocalin the answer are not present whenwith-localis not active. I chose to set default values in the javascript object response, but it could also be done the same way with no values present at all. Which would be best ?