Gargron / fileupload

PHP FileUpload library that supports chunked uploads
MIT License
458 stars 87 forks source link

Adding basic callbacks support #2

Closed thers closed 11 years ago

thers commented 11 years ago

Now only for upload complete event.

Gargron commented 11 years ago

I like this! The way I was doing it in my application before was looping through the $files received from processAll, checking if $file->path is set, and then doing something. This is more elegant. Lacks test coverage at the moment, and there's a comment I left about catching callback exceptions. Otherwise this is good to go. If you could write a test for this before merging it would be sweet (otherwise Travis might not even catch on if the callback support breaks anything)

thers commented 11 years ago

Well, after some hesitation, I have come to believe that you are right about catching exceptions isn't a good idea, devs can take care of it yourself :)

thers commented 11 years ago

And yeah, i should write some tests, but not for now, now it's actually written with thought that callbacks can not actually break something :D

Gargron commented 11 years ago

Okay then, just remove the try/catch and I'll merge this in :)

thers commented 11 years ago

Don't know if Github sent some notification about newly added commit, so this comment will be here just for this %)

Gargron commented 11 years ago

@RiderSx Hey, I added related info to the README, added a test, and also did some little changes to your code, see https://github.com/Gargron/fileupload/commit/e309f2ed5c89025193f89f3210934a7be702c08a

thers commented 11 years ago

@Gargron Hi, yeah, I would have to add it by myself, excuse me for the haste, i was in process to replace default jQuery-file-upload handler with this one, and my patch had to be done to the end of the day :)