-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
}) | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to self and other readers, tests were moved to ipfs-inactive/interface-js-ipfs-core#267
ba8c4c8
to
34d9881
Compare
Please rebase master onto this branch for green CI |
34d9881
to
41a0d49
Compare
This has been rebased w/ the version of #1306 that was rebased against master. Tests probably won't pass without the changes to js-ipfs-api in place |
src/core/components/bitswap.js
Outdated
}, | ||
let list | ||
if (peerId) { | ||
peerId = PeerId.createFromB58String(peerId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A try/catch around this will allow us to callback with an error for invalid user input.
src/cli/commands/block/get.js
Outdated
process.stdout.write(block.data) | ||
} else { | ||
process.stderr.write('Block was unwanted before it could be remotely retrieved') | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we'd use print
from utils so that output can be disabled (I'm not sure why you'd want to do that for this call but we should probably support it!).
src/core/components/bitswap.js
Outdated
} | ||
|
||
// TODO: implement when https://github.com/ipfs/js-ipfs-bitswap/pull/10 is merged | ||
} | ||
callback(null, self._bitswap.unwant(key)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setImmediate
here too?
src/core/components/bitswap.js
Outdated
list = self._bitswap.getWantlist() | ||
} | ||
list = formatWantlist(list) | ||
callback(null, list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setImmediate
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed there's missing tests for the bitswap CLI, we need a test for wantlist with a peer and a test for unwant.
Could you also enable testing of the http interface with interface-ipfs-core
by dropping a bitswap.js
file in here.
Awesome work BTW 🚀
@wraithgar whats the status on this? Can I do to help you pull this over the line? |
5953a59
to
371bb56
Compare
Ok the rebase and test updates are done. Updated original comment to link to API PR. Still needs the CLI tests but everything else seems to be in place and working with both go (via the api tests) and js (via these tests) |
CLI tests added and bugs found therein fixed. |
@wraithgar there seems to be linting and build issues still with this - would you mind taking a look? |
afc9ba3
to
2733a12
Compare
@alanshaw Linting errors fixed. Build errors are because this depends on the PRs in both the API and Interface. |
2733a12
to
8be98df
Compare
8be98df
to
0c36ab5
Compare
@wraithgar FYI for the future, to appease the commit linter your commit message prefixes need a colon after them |
Will require including the version of interface-ipfs-core that they are moved to
Will require including the version of interface-ipfs-core that they are moved to
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
1e9dc25
to
d5346cd
Compare
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
It green! |
Because of the amount of work done in #1306 this PR is built off of that, that PR needs to be merged before this one can be isolated and discussed.
This adds a peerId parameter to bitswap.unwant.
Tests are in ipfs-inactive/interface-js-ipfs-core#267
API changes are in ipfs-inactive/js-ipfs-http-client#761