chore: add upload of a blob file #86
Conversation
lib/vimeo.js
Outdated
| params = {} | ||
| } | ||
|
|
||
| var fileSize = file.size |
There was a problem hiding this comment.
To eliminate a lot of boilerplate and duplicated functionality, we could probably merge this line with line 436 and if file is a file object/blob, run this, else pass it through fs.statSync.
There was a problem hiding this comment.
Also if you don't mind, could you also mimic the same behavior within .replace so it can accept either a file path or a blob?
There was a problem hiding this comment.
@erunion for sure sr, I will make this change. :)
There was a problem hiding this comment.
Also if you don't mind, could you also mimic the same behavior within
.replaceso it can accept either a file path or a blob?
@nicolastakashi I just noticed that you forgot to include this one on your most recent revision.
Also, to anyone, is there any way I could change my local copy to work while waiting for the PR to get merged?
lib/vimeo.js
Outdated
| progressCallback, | ||
| errorCallback | ||
| ) { | ||
| var upload = new tus.Upload(file, { |
There was a problem hiding this comment.
Similar to the other comment, we can eliminate this method and merge it with _performTusUpload by detecting if file is a blob or string, and running it through fs.createReadStream depending.
There was a problem hiding this comment.
Great! It's make sence, I will make the change
|
Hi, any news on this? Will this enable me to upload videos from my client SPA using |
|
@josephharveyangeles Yes, but notice that with the new method
|
|
Hi guys, sorry for my delay. I had some difficulties and I coun't fixe the code that @erunion told me to fixe. I'll to try fixe that until tomorrow and I let you know about that. |
|
@nicolastakashi Thank you for your work on this. I just tried and works wonderfully. I'd like to help but I'm not that good at JS, but learning from your code! Meanwhile I'll create my own fork and merge your PR there. Again, thanks! |
|
Hey mate! I just finish to fixe the code review. Now I'll to make some tests and let you know about that. tks |
|
I tested this just now, and it worked like a charm. When this PR got merged to master, on what version should would these changes reflect? I'm assuming the CI will automatically publish it on npm right? |
|
@josephharveyangeles
The safest approach would be to use a fork. You can create your own or use Nicola's. If you create your own, you'll have to merge Nicola's change yourself, either to Then, in your package manager simply use the git remote, instead of the library version: "vimeo": "https://github.com/GITHUB_ACCOUNT/vimeo.js#YOUR_BRANCH" |
|
Hey you mates. I just push a new version and I change the I made some tests but I will appreciate if someone could make more tests to ensure the functionality |
Co-Authored-By: nicolastakashi <nicolas.tcs@hotmail.com>
Co-Authored-By: nicolastakashi <nicolas.tcs@hotmail.com>
Co-Authored-By: nicolastakashi <nicolas.tcs@hotmail.com>
Co-Authored-By: nicolastakashi <nicolas.tcs@hotmail.com>
Co-Authored-By: nicolastakashi <nicolas.tcs@hotmail.com>
|
Great Sr! Was a pleasure to talk with you :) If exists some other change that I could to do, please let me know <3 |
|
Fantastic. I just released this to 2.1.1. Thanks @nicolastakashi, and everyone! |
|
@jeffreylanters I believe you're referring to the |
|
@erunion I see, we we're using @nicolastakashi's Fork. He should pull your fix into his repository then. Thanks for the response! Edit: I just see his Fork is already merged into the master. We'll be fine using the master. Thanks again! |
resolve issue #80