-
Notifications
You must be signed in to change notification settings - Fork 109
refactor: convert bitswap tests to use async/await #391
Conversation
NOTE: the PRs for ipfs/js-ipfs#1670 are being merged into the `feat/async-iterators` branch. Once they're all in we can merge them into master. --- This simply refactors the bitswap tests to use async/await and updates the documentation to remove callback support and propose a better method for documenting the parameters and return values. depends on #390 License: MIT Signed-off-by: Alan Shaw <[email protected]>
**Example:** | ||
| Type | Description | | ||
|------|-------------| | ||
| `Promise<{`<br/> `Keys<Array<Object>>`<br/>`}>` | The list of CIDs wanted by the peer. Each object in the array has a single property "/" a string CID. | |
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.
this seems weird, html inside inline code block just to add new lines and space
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 know 😢. I would like this to be clearer when viewed as markdown but I don't know how right now. Unfortunately multi-line code blocks in a table aren't possible. If you have any ideas for how to format this in markdown so that it reads clearly when rendered then please comment!
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've recently also started to use tables for parameters (for js-ipld). But I'll move away from that again. I find tables hard to read if you just read the unrendered Markdown.
The signature is quite complicated, I actually missed that it is an object containing a single field called Keys
. I found that clearer in the old docs.
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've recently also started to use tables for parameters (for js-ipld). But I'll move away from that again. I find tables hard to read if you just read the unrendered Markdown.
👍
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.
Ok, I will think up a better way to layout this information that is readable rendered or not.
|
||
// { Keys: [{ '/': 'QmHash' }] } | ||
```js | ||
const list = await ipfs.bitswap.wantlist() |
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.
Should this return an iterator instead of a 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.
I think in general we should either return single values or iterators of single values, never lists of single values.
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 agree, the wantlist could get really big so an iterator makes sense.
never lists of single values
Do you think this should always be true? I think we should return async iterators where it makes sense to "stream" the output but I wouldn't have thought it would be necessary for something like ipfs.bootstrap.list
, for example...but could be persuaded for the sake of consistency.
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.
we talking iterable ? or really async iterator ?
superseded by #547 |
NOTE: the PRs for ipfs/js-ipfs#1670 are being merged into the
feat/async-iterators
branch. Once they're all in we can merge them into master.This simply refactors the bitswap tests to use async/await and updates the documentation to remove callback support and propose a better method for documenting the parameters and return values.
depends on #390