pinning API #34
pinning API #34
Conversation
| @@ -0,0 +1,73 @@ | |||
| pinning API | |||
| ##### `JavaScript` - ipfs.pin.rm(hash, [callback]) | ||
|
|
||
| Where `hash` is a multihash. | ||
|
|
There was a problem hiding this comment.
What about recursive? Should be a default true bool.
|
|
||
| describe('callback API', () => { | ||
| it('.remove (first, because adding is recursive)', (done) => { | ||
| const hash = 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP' |
There was a problem hiding this comment.
Make this const declared above, not sure why you declare it so many times.
There was a problem hiding this comment.
I kind of like of having tests that are really self descriptive, it makes it easier to read and to change.
| ipfs.pin.rm(hash, { recursive: true }, (err, res) => { | ||
| expect(err).to.not.exist | ||
| expect(res).to.exist | ||
| ipfs.pin.ls({ type: 'direct' }, (err, res) => { |
There was a problem hiding this comment.
Try with all different types?
| ```JavaScript | ||
| { | ||
| hash: 'QmHash', | ||
| path: 'some-path |
|
Thank you @RichardLitt :D Applied almost all your CR, will add a couple more tests to cover remaining cases :) |
| @@ -0,0 +1,91 @@ | |||
| Pin API | |||
There was a problem hiding this comment.
The folder is called pinning-api, so this probably should be called Pinning API
daviddias
force-pushed
the
pinning-api
branch
3 times, most recently
from
4730c36
to
981f803
Compare
|
Applied all the CR :) |
| Where: | ||
| - `hash` is a multihash. | ||
| - `options` is an object that can contain the following keys | ||
| - `recursive` - Recursively unpin the object linked. Type: bool. Default: 'false' |
There was a problem hiding this comment.
Richard disagreed on that one, I'll let that 🚲🏠 happen on a separate PR
There was a problem hiding this comment.
@RichardLitt why? (can't find the comment), 'false' in a markdown document looks like it is a string.
There was a problem hiding this comment.
Yeah, that looks fine to me. This should be default true, though!
| expect(Object.keys(pinset).length > 0).to.equal(true) | ||
| done() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
These three tests above could be simplified to
function testLs (type) {
return it(`.ls type ${type}`, (done) => {
ipfs.pin.ls({type}, (err, pinset) => {
expect(err).to.not.exist
expect(Object.keys(pinset)).to.be.not.empty
done()
})
})
}
testLs('recursive')
testLs('indirect')
testLs('direct')There was a problem hiding this comment.
Simplified, but not simpler for changes, reading, etc :)
There was a problem hiding this comment.
fair enough, but please use .not.empty rather than .length > 0).equal(true)
|
lgtm |
Works in js-ipfsmilestone 6