dwyl / esta

:mag: Simple + Fast ElasticSearch Node.js client. Beginner-friendly defaults & Heroku support :white_check_mark: :rocket:

48 stars 10 forks source link

Tidy up http_request ? #81

Closed nelsonic closed 9 years ago

nelsonic commented 9 years ago

No need to return the request object. https://github.com/nelsonic/esta/blob/40c4ef3016508385ba4537aa9facc770cc734452/lib/http_request.js#L31 Could we simplify this?

FilWisher commented 9 years ago

As far as I can tell, the request stream is returned so you can .write() to the request body? Both shot and request pass request body into options.

If the logic of writing this body from options to the request stream is contained within http_request, the performance shouldn't change and it will have a slightly more straightforward API.

Happy to PR if you agree

nelsonic commented 9 years ago

mindreader :wink:

PR #FTW :+1:

FilWisher commented 9 years ago

82 - Happy to make additional changes

nelsonic commented 9 years ago

Just need to version bump to 4.0.3 and add your name to package.json http://browsenpm.org/package.json#contributors :+1:

nelsonic commented 9 years ago

Thanks @FilWisher :8ball: @iteles to review/merge. :tada:

iteles commented 9 years ago

Done! :+1: