Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

ronag
Copy link
Member

@ronag ronag commented Jun 1, 2020 •
edited

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 mcollina and addaleax June 1, 2020 16:40
@nodejs-github-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Jun 1, 2020
@ronag
Copy link
Member Author

ronag commented Jun 1, 2020

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
@ronag ronag changed the title [v14.x] revert 28e6626ce7020b490438e3ee8a8188a59c5f856f [v14.x] revert set IncomingMessage.destroyed Jun 1, 2020
@ronag ronag mentioned this pull request Jun 1, 2020
Closed
4 tasks
@addaleax
Copy link
Member

addaleax commented Jun 1, 2020

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
Copy link
Collaborator

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

@richardlau
Copy link
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.

@ronag ronag 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
Copy link
Member

@mcollina mcollina left a comment

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
Copy link

@willin willin left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina
Copy link
Member

mcollina commented Jun 3, 2020

@ronag you likely need to rebase this

@ronag ronag force-pushed the v14.x-revert-28e6626ce7020b490438e3ee8a8188a59c5f856f branch from 50b85e2 to d3b3684 Compare June 3, 2020 10:30
@ronag
Copy link
Member Author

ronag commented Jun 3, 2020

rebased

@mcollina
Copy link
Member

mcollina commented Jun 9, 2020

cc @BethGriggs

@ronag
Copy link
Member Author

ronag commented Jun 27, 2020

Added PR-URL

@ronag ronag force-pushed the v14.x-revert-28e6626ce7020b490438e3ee8a8188a59c5f856f branch from d3b3684 to 37d9cee Compare June 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
Copy link
Member

Landed in 6dbd63c

@codebytere codebytere closed this Jun 27, 2020
@codebytere codebytere 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@addaleax addaleax addaleax approved these changes

@willin willin willin approved these changes

@codebytere codebytere codebytere approved these changes

@cjihrig cjihrig cjihrig approved these changes

@juanarbol juanarbol juanarbol approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http Issues or PRs related to the http subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@ronag @addaleax @nodejs-github-bot @richardlau @mcollina @codebytere @willin @cjihrig @juanarbol