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

Return full response #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Stephenitis
Copy link

I had a need to process the response headers in the response from a successful post/get request.
Callback variables now looks like (err, res, body)

@ahallock
Copy link
Owner

Thanks. I'll have time to test this later today.

@ahallock
Copy link
Owner

A few things:

  1. It breaks 48/50 tests, so I can't merge yet.
  2. It breaks current code out there since the body is being moved one position.

This isn't 1.0 yet, so we can break the current contract, but we should increment to 0.5.0 if we do.

But this change makes sense and is consistent with Request's fn(err, res, body). Can you fix the tests?

Thanks

@Stephenitis
Copy link
Author

cool, this is my first attempted pull on a node module, I'll submit a
revised one soon.

On Mon, Dec 23, 2013 at 7:32 PM, Andrew Hallock notifications@github.comwrote:

A few things:

  1. It breaks 48/50 tests, so I can't merge yet.
  2. It breaks current code out there since the body is being moved one
    position.

This isn't 1.0 yet, so we can break the current contract, but we should
increment this to 0.5.0 if we do. I do think this change makes sense and is
consistent with Request's fn(err, res, body).


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-31156837
.

Stephen Nguyen
Developer Evangelist, Iron.io http://www.iron.io/
703-975-5863 | stephen@iron.io
http://www.linkedin.com/in/stephenhnguyen/ http://www.github.com/stephenitis
http://www.twitter.com/stephenitis

Developers <3 Iron.io! https://twitter.com/getiron/favorites

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.

None yet

2 participants