Skip to content

[v14.x] revert set IncomingMessage.destroyed - #33686

Closed
ronag wants to merge 1 commit into
nodejs:v14.x-stagingfrom
nxtedition:v14.x-revert-28e6626ce7020b490438e3ee8a8188a59c5f856f
Closed

[v14.x] revert set IncomingMessage.destroyed #33686
ronag wants to merge 1 commit into
nodejs:v14.x-stagingfrom
nxtedition:v14.x-revert-28e6626ce7020b490438e3ee8a8188a59c5f856f

Conversation

@ronag

@ronagronag commented Jun 1, 2020•
edited
Loading

Copy link
Copy Markdown
Member

This reverts commit 28e6626.

After more thought #33591 I think the consensus is that this should have landed as semver-major. Also requires some more PR's to land to work well. #33655#33654

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@ronag
ronag requested review from addaleax and mcollinaJune 1, 2020 16:40
@nodejs-github-botnodejs-github-bot added the http Issues or PRs related to the http subsystem. label Jun 1, 2020
@ronag

ronag commented Jun 1, 2020

Copy link
Copy Markdown
MemberAuthor

First time creating a revert PR so feedback is welcome on commit message, PR naming etc...

@ronag

This comment has been minimized.

Sign in to view
@ronagronag changed the title [v14.x] revert 28e6626ce7020b490438e3ee8a8188a59c5f856f[v14.x] revert set IncomingMessage.destroyed Jun 1, 2020
@ronagronag mentioned this pull request Jun 1, 2020
Closed
4 tasks
@addaleax

addaleax commented Jun 1, 2020

Copy link
Copy Markdown
Member

I'm missing 14.x in the rebase drop down?

It is missing, yes, but <pr base branch> will do the right thing anyway.

@nodejs-github-bot

nodejs-github-bot commented Jun 1, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31688/

@richardlau

richardlau commented Jun 1, 2020

Copy link
Copy Markdown
Member

I'm missing 14.x in the rebase drop down?

It is missing, yes, but <pr base branch> will do the right thing anyway.

I think these are there for things like security releases (where the release is prepped in the private repo). Anyway I've added the 14.x branches (v14.x and v14.x-staging) to the list of options in case it's needed for this week's security releases.

@ronagronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 1, 2020
mcollina
mcollina approved these changes Jun 1, 2020
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

willin
willin approved these changes Jun 3, 2020
View reviewed changes

@willinwillin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@mcollina

mcollina commented Jun 3, 2020

Copy link
Copy Markdown
Member

@ronag you likely need to rebase this

@ronag
ronagforce-pushed the v14.x-revert-28e6626ce7020b490438e3ee8a8188a59c5f856f branch from 50b85e2 to d3b3684CompareJune 3, 2020 10:30
@ronag

ronag commented Jun 3, 2020

Copy link
Copy Markdown
MemberAuthor

rebased

@mcollina

mcollina commented Jun 9, 2020

Copy link
Copy Markdown
Member

cc @BethGriggs

@ronag

ronag commented Jun 27, 2020

Copy link
Copy Markdown
MemberAuthor

Added PR-URL

@ronag
ronagforce-pushed the v14.x-revert-28e6626ce7020b490438e3ee8a8188a59c5f856f branch from d3b3684 to 37d9ceeCompareJune 27, 2020 16:40
codebytere pushed a commit that referenced this pull request Jun 27, 2020
@ronag@codebytere
Revert "http: set IncomingMessage.destroyed"
6dbd63c
This reverts commit 28e6626.
PR-URL: #33686
Backport-PR-URL: #33686
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@codebytere

codebytere commented Jun 27, 2020

Copy link
Copy Markdown
Member

Landed in 6dbd63c

@codebyterecodebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@ronag@codebytere
Revert "http: set IncomingMessage.destroyed"
7fdb507
This reverts commit 28e6626.
PR-URL: #33686
Backport-PR-URL: #33686
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@addaleaxaddaleaxaddaleax approved these changes
@codebyterecodebyterecodebytere approved these changes
@cjihrigcjihrigcjihrig approved these changes
@juanarboljuanarboljuanarbol approved these changes
+1 more reviewer
@willinwillinwillin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.httpIssues or PRs related to the http subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@ronag@addaleax@nodejs-github-bot@richardlau@mcollina@codebytere@willin@cjihrig@juanarbol