Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

add data and response into request #149

Merged
merged 1 commit into from Mar 12, 2017
Merged

Conversation

watilde
Copy link
Contributor

@watilde watilde commented Jan 10, 2017

from npm/npm#15306. To get a statusCode in npm/lib/ping.js, we need to pass them even error exists.

@zkat zkat removed the review label Mar 7, 2017
Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @watilde -- do you mind writing a quick test to https://github.com/npm/npm-registry-client/blob/master/test/ping.js to make sure we're returning everything even on error?

@watilde watilde force-pushed the feature/status branch 3 times, most recently from 104e031 to 761aef9 Compare March 10, 2017 01:00
@watilde
Copy link
Contributor Author

watilde commented Mar 10, 2017

@zkat Hello! I just added a test for it now :)

@zkat
Copy link
Contributor

zkat commented Mar 10, 2017

@watilde thanks! Could you make sure data and response are part of the tests, too? This was helpful, thank you ❤️

@watilde
Copy link
Contributor Author

watilde commented Mar 12, 2017

Ah you're right, that should be checked in tests :)

I've added the following two lines for it!

+    t.same(data, JSON.stringify(wanted))
+    t.same(res.body.toString(), JSON.stringify(wanted))

Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪🏼

@zkat zkat merged commit ed0236e into npm:master Mar 12, 2017
@zkat
Copy link
Contributor

zkat commented Mar 12, 2017

Released as npm-registry-client@7.5.0

@watilde watilde deleted the feature/status branch March 12, 2017 22:14
@watilde
Copy link
Contributor Author

watilde commented Mar 12, 2017

Great! Arigato<3

@zkat
Copy link
Contributor

zkat commented Mar 13, 2017

いいえいいえ。こちらこそ、ワチくん。^___^

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

Successfully merging this pull request may close these issues.

None yet

3 participants