-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added writeStream support for uploadSession #31
base: master
Are you sure you want to change the base?
Conversation
Removed promise execution
} | ||
params.readableStream.resume(); | ||
}).catch(e => { | ||
console.log('err') |
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.
can we remove log
*/ | ||
|
||
function uploadSession(params, progress = () => {}) { | ||
function uploadSession(params, progress, writeStream, cb) { |
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.
progress
is failing for me as it is required now. If you rebase from master it will default to progress = () => {}
and wont break existing code
This looks great, thanks! Would you mind writing an e2e test for a writable stream? There is a documentation for e2e tests (lacking) here ./test and example from @namdien177 #29 I can help you out with writing it (or write it if you are stuck / out of free time) |
Fixes #28