sendAnonymousCLIMetrics added #139
Conversation
|
Finally, FINALLY, we're ready to take this! =D |
| var assert = require('assert') | ||
| var url = require('url') | ||
|
|
||
| function send (uri, params, cb) { |
There was a problem hiding this comment.
This function should compose its URL on its own (for example how lib/dist-tags/set.js works).
|
|
||
| function nop () {} | ||
|
|
||
| var URI = 'https://npm.registry:8043/-/npm/anon-metrics/v1/:metricId' |
There was a problem hiding this comment.
:metricId shouldn't be passed in literally, it's supposed to be replaced with the metricId value from the document we're given.
| ) | ||
|
|
||
| params.method = 'PUT' | ||
| this.request(uri, params, cb) |
There was a problem hiding this comment.
Params here can't just be passed through to request. What you're passing in the test isn't valid for this.request. Again, see lib/dist-tags/set.js, they would need to be JSON encoded and put into a body property.
|
hehe ok that's good, I'll have a look at the changes requested. If you're going to be faster, I will just compare the results and see how this should've been done. Thanks nonetheless. |
Fixes: #138 Credit: @lazywithclass Credit: @iarna Reviewed-By: @iarna PR-URL: #139
|
Merged as fc7b81b |
Fixes: #138 Credit: @lazywithclass Credit: @iarna Reviewed-By: @iarna PR-URL: #139
Fixes: #138 Credit: @lazywithclass Credit: @iarna Reviewed-By: @iarna PR-URL: #139
Fix a bug in fetch where errors on the request object that came after the response object had been emitted could result in duplicate callbacks and suppressed error conditions. Ultimately this translates to fixing shasum mismatches in `npm` when they were associated with an `ECONNRESET` or other network error. PR-URL: npm/npm-registry-client#139 Fixes: #14626 Credit: @iarna Add support for sending anonymous cli metrics. PR-URL: npm/npm-registry-client#148 Fix support for sending anonymous cli metrics. Credit: @sisidovski PR-URL: npm/npm-registry-client#147
Send metrics to the registry as per #138