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

Emitting send and end events at json call #61

Closed
wants to merge 4 commits into from
Closed

Conversation

ti0ma
Copy link

@ti0ma ti0ma commented May 8, 2015

I saw that this have been implemented at #24 some time ago, but without tests.
As I need this features like water, I used the code at this pull request and implemented a test.

I've seen that there aren't tests for response.json() method but i've not implemented them all, because it's not the goal of this pull request. But I actually can do this if it's necessary to move this feature forward.

Cheers!

@estilles
Copy link

estilles commented May 8, 2015

@ti0ma, thanks for the PR. I wasn't going to entertain these types of requests until after the 2.0 rewrite, but if you really need this I will merge it. Before I merge, I would ask that you squash your commits and resubmit. Once you do I will gladly merge into the 1.x branch and publish to npm.

Thanks again and sorry for the inconvenience.

@ti0ma
Copy link
Author

ti0ma commented May 11, 2015

I've solved this issue pushing my fork to my private npm registry, so I don't need this at the official repo as well.

But if someone needs it, I squashed the commits and pushed again.

Thanx for all,
Cheers!

@estilles
Copy link

@ti0ma ... thanks again. If you no longer have any urgency for this change I would prefer to hold off on merging it. This issue is already address in the upcoming 2.0 release. If you (or anyone else for that matter) find themselves needing this with any degree of urgency, just ping me and I will gladly merge it.

@estilles
Copy link

estilles commented Jun 2, 2015

There appears to still be some immediate interest in this PR. I will merge this into the 1.x branch and release in 1.4.3 tonight.

@estilles
Copy link

estilles commented Jun 3, 2015

Merged via ae6c785

@estilles
Copy link

estilles commented Jun 3, 2015

Published in release 1.4.3

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

Successfully merging this pull request may close these issues.

2 participants