[@types/node] Add missing method property to http.ClientRequest#41029
[@types/node] Add missing method property to http.ClientRequest#41029andrewbranch merged 9 commits intoDefinitelyTyped:masterfrom
method property to http.ClientRequest#41029Conversation
Because it gets set in that order in the source code.
There is nothing in the source code that prevents this property from being overwritten, and user code and/or modules may change this property before flushing the HTTP header.
|
@TooTallNate Thank you for submitting this PR! 🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @jeremiergz - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
galkin
left a comment
There was a problem hiding this comment.
@TooTallNate, thank you for your work. Could you update node v10 also with the same changes?
|
Sure thing. Pushed to v10 as well. |
|
@TooTallNate One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits or comments. Thank you! |
|
👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings. Let’s review the numbers, shall we? Comparison details 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
|
@TooTallNate, there are two pull: this and that #41030. I think you pushed to wrong one. |
|
@TooTallNate I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues. |
|
Merged #41030 into this one. |
|
🔔 @galkin - Thanks for your review of this PR! Can you please look at the new code and update your review status if appropriate? |
|
A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped! |
|
I just published |
Please fill in this template.
npm test.)npm run lint package-name(ortscif notslint.jsonis present).Select one of these and delete the others:
If changing an existing definition: