add progress event #107
add progress event #107
Conversation
LinusU
left a comment
LinusU
left a comment
There was a problem hiding this comment.
I think that we should give a bit more thought to what goes in to the progress event. Could we find any examples of similar modules emitting similar events? I think it would be good to see what the rest of the ecosystem is doing...
package.json
Outdated
| { | ||
| "name": "speaker", | ||
| "version": "0.4.0", | ||
| "version": "0.4.1", |
There was a problem hiding this comment.
Please don't bump the version in the PR, that's better left to the release process
There was a problem hiding this comment.
Thanks for the replies.
For the version bump, I've worked on other projects where they did that. I'll take it out.
I was thinking that the call-back would loose context (I haven't worked with libuv much), but I can remove "THIS" if it's not needed.
For the naming conventions, there are a lot of styles out there. Is there an example style that I should use? WRT the actual data fields included in the event, I just put in there what I needed for my little use case, but other data can be added if appropriate. Or should it not put include any data, and then the receiver can just pull the data out of the object?
index.js
Outdated
| binding.write(handle, b, b.length, onwrite) | ||
| } | ||
|
|
||
| var THIS = this; //preserve "this" for onwrite call-back |
There was a problem hiding this comment.
You don't need this variable since onwrite is an arrow-function
| * `numwr` - the cumulative number of writes (this is related to the number of frames) | ||
| * `wrlen` - the number of bytes written this time | ||
| * `wrtotal` - the total number of bytes written | ||
| * `buflen` - the number of bytes currently remaining in the buffer |
There was a problem hiding this comment.
Could we try and name these something more human friendly?
There was a problem hiding this comment.
Actually it would be a kind of acknowledgement or ack .
3 participants
Added event to send progress info back to caller.