Skip to content
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

BeforeRequest and AfterCommit Event #50

Closed
wants to merge 4 commits into from
Closed

BeforeRequest and AfterCommit Event #50

wants to merge 4 commits into from

Conversation

rgolea
Copy link
Contributor

@rgolea rgolea commented Jan 13, 2016

I've finally managed to have time to rewrite everything and make it so you can merge it nicely. Test it and give me a feedback.

@NicolasRitouet NicolasRitouet self-assigned this Jan 13, 2016
@israelroldan
Copy link

If #47 gets merged this will most likely come there as well as these two events will be delegated to the underlying collection (Coincidentally I just sent PR #1 against EvilDrW's fork)

@rgolea
Copy link
Contributor Author

rgolea commented Jan 18, 2016

Sure! That'd be great... Just let me know when it's going to happen and what will the new api be for events... I'm using the module in production so I really want it to not go far away from the master branch of this project. Right now, I'm using my own.

@israelroldan
Copy link

ok, so it seems that my PR to EvilDrW's fork didn't make it in time so the availability of AfterCommit and BeforeRequest is not there yet (even now that #47 has just been merged). Given that you're the owner of this PR, could I suggest you a different implementation approach? Now that FIleUpload inherits from Collection it's just a matter of exposing the events in the Fileupload.events array on line 94 of index.js, the underlying collection can take care of the rest.

@rgolea
Copy link
Contributor Author

rgolea commented Jan 18, 2016

I'm not really sure what to do next. My entire project is currently depending on this module as it was. In order to use that PR, I need all the events that the collections can bring me because I was using a this module with a normal collection based on the "fileupload" suffix. I might not make the changes in the end. @israelroldan, try to send yourself a PR since I really don't have time to implement it. I might be able to do some workaround later on...

@rgolea rgolea closed this Jan 18, 2016
@rgolea
Copy link
Contributor Author

rgolea commented Jan 18, 2016

PS: Besides, the current folder name has changed as well...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants