Xuanwo commented on code in PR #3619:
URL:
https://github.com/apache/incubator-opendal/pull/3619#discussion_r1405888820
##########
bindings/nodejs/index.js:
##########
@@ -19,9 +19,59 @@
/// <reference types="node" />
+const { Writable, Readable } = require('node:stream')
+
+class ReaderStream extends Readable {
Review Comment:
> Their name can't really reflect blocking. the stream itself is not
blocking.
I know nodejs has an event loop, and I know it's not blocking even when we
call `readerSync`. But that doesn't mean we don't need to add both `ReadStream`
and `BlockingReadStream`.
All our API have two style, one for `Sync` and other one for `Async`. And
they will return two different class `BlockingReader` and `Reader`. So it's
quiet weird that `BlockingReader` has an API `createReadStream` but `Reader`
doesn't have.
So it's not an issue that we *need* or not, it's about our API design: Do we
need to have same feature set for our public structs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
