4.0.0 #82
4.0.0 #82
Conversation
Overall it is a bad practice to 'fix' internal modules within the other module. I think graceful-fs (https://github.com/isaacs/node-graceful-fs#global-patching) is good example, how to do this: ```js var http = require('http'); var https = requrie('https'); var infinityAgent = require('infinity-agent'); infinityAgent.patch(http); infinityAgent.patch(https); ``` This will enable global agent tuning in edge cases. Closes #73 and closes #60
* Base logic moved into event-emitter and all APIs wrapped around it * read-all-stream combined into one place - less code duplication * Unzip stream now inherits all properties and function from response and returned instead of response Closes #66
| unzip.setTimeout = res.setTimeout.bind(res); | ||
| unzip.statusCode = res.statusCode; | ||
| unzip.statusMessage = res.statusMessage; | ||
| unzip.socket = res.socket; |
There was a problem hiding this comment.
It will copy internal fields and break unzip stream. Maybe turn it to a module?
> EventEmitter was made to be used as an abstraction around asynchronous events. Thus, when an event is emitted it will have happened on a different tick than when the event was set. See nodejs/node-v0.x-archive#8470 (comment)
|
@sindresorhus friendly ping :) |
|
Sorry about the delay. I'm traveling in Iceland at the moment. Will be back on Friday. |
| got.HTTPError = createErrorClass('HTTPError', function() { | ||
| this.message = 'Response code ' + this.statusCode + ' (' + http.STATUS_CODES[this.statusCode] + ')'; | ||
| this.statusMessage = http.STATUS_CODES[this.statusCode]; | ||
| }); |
There was a problem hiding this comment.
Can be simplified:
Also the space after function
got.HTTPError = createErrorClass('HTTPError', function () {
this.statusMessage = http.STATUS_CODES[this.statusCode];
this.message = 'Response code ' + this.statusCode + ' (' + this.statusMessage + ')';
});
|
Ok, found some time on the airport to review. Expect for some super minor nitpick this looks really good! I like how the code is a lot more readable now. Great work :) Feel free to merge when the inline comments are resolved. |
|
I feel like some more time should be spent on Errors (because of #86, #72 and floatdrop/create-error-class#1), so new Errors will move to another PR. @sindresorhus thanks for kind words ✨ I really miss 80-lines version of got, so tried to simplify things as much as I can. |
|
Super cool work, @floatdrop! ⭐ |
|
@floatdrop Anything else we want to get into 4.0.0? Breaking changes would be nice to get in now so 5.0.0 can be far in the future. |
|
@sindresorhus nothing, except new Errors (#88). |
🐲 Here be dragons!
Promise API
As discussed in #66.
pinkie-promisewas taken as fallback for native Promise.infinity-agent removed
Main reason to use
infinity-agentwas to fix internalmaxSocketsproperty on 0.10 version of NodeJS. But this approach is not quite configurable and not quite composable with custom agent, which could be necessary to fix #79So
infinity-agentshould be dropped in favore to readme section with workaround: