FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Revert "http: make HEAD method to work with keep-alive" by targos · Pull Request #38949 · nodejs/node · GitHub

/ node Public

Revert "http: make HEAD method to work with keep-alive" - #38949

Closed
targos wants to merge 1 commit into
nodejs:masterfrom
targos:fix-38922
ConversationCommits (1)ChecksFiles changed
Closed

Revert "http: make HEAD method to work with keep-alive"#38949
targos wants to merge 1 commit into
nodejs:masterfrom
targos:fix-38922

Conversation

targos commented Jun 6, 2021

Copy link
Copy Markdown
Member

This reverts commit 7afa533.

The change breaks clients like cURL.

Fixes: #38922

Revert "http: make HEAD method to work with keep-alive"
dce7bb2 
This reverts commit 7afa533.

The change breaks clients like cURL.

Fixes: nodejs#38922
github-actions Bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jun 6, 2021
targos mentioned this pull request Jun 6, 2021
Closed
mcollina approved these changes Jun 6, 2021
View reviewed changes

mcollina 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

targos commented Jun 6, 2021

Copy link
Copy Markdown
Member Author

Hi, @bl-ue and @VoltrexMaster! I appreciate all your comments in the issue tracker, but it would be helpful if you could refrain from "approving" pull requests. Overall, it causes a lot of notification noise. A thumbs-up reaction would convey the same information without piling up notifications for collaborators.

Given how much you both watch and interact with the repository, I want to make sure you are aware of our triager role, in case that is of interest to you.

bl-ue commented Jun 6, 2021

Copy link
Copy Markdown
Contributor

Hi @targos! Sure, I'll avoid doing that - I didn't realize it was bothering you :)
I'll take a look at that repo and get started helping. I'd like to be a triager, but I don't want my name or email anywhere obvious like on the readme, and I'd also like to avoid any meetings - is this possible? If it's not, I'll just live without the extra power, but it would be cool! :)
I'll definitely help out though.

Trott commented Jun 7, 2021

Copy link
Copy Markdown
Member

I'd like to be a triager, but I don't want my name or email anywhere obvious like on the readme, and I'd also like to avoid any meetings - is this possible?

As far as I know, there are no meetings for triagers, so I think that's not a problem.

Name is also probably not a problem. We allow people to put whatever name they want (although we'd probably step in and block anything that was intentionally misleading, like using the name of an existing collaborator, or something intentionally and blatantly offensive).

Email, I'm less sure about. We need a way to get in touch with people outside of GitHub sometimes, and that is usually through the email listed in the README.

bl-ue commented Jun 7, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Email, I'm less sure about. We need a way to get in touch with people outside of GitHub sometimes, and that is usually through the email listed in the README.

Understood, so maybe I'll just have to decline — but I'm available all day every day at https://gitter.im/bl-ue ;)

mhdawson approved these changes Jun 7, 2021
View reviewed changes

mhdawson 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

targos commented Jun 7, 2021

Copy link
Copy Markdown
Member Author

@nodejs/tsc @nodejs/lts Does anyone object to pulling this into v14.17.1, to avoid having the change in LTS for too long?

richardlau commented Jun 7, 2021

Copy link
Copy Markdown
Member

No objections to including in 14.17.1 from me.

mcollina commented Jun 7, 2021

Copy link
Copy Markdown
Member

no objection, actually it's the course of action that I would recommend.

mmarchini approved these changes Jun 7, 2021
View reviewed changes

mmarchini left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

and +1 on pulling it on 14.17.1

nodejs-github-bot commented Jun 8, 2021

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jun 8, 2021

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jun 8, 2021

Copy link
Copy Markdown
Collaborator

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

mcollina pushed a commit that referenced this pull request Jun 8, 2021
Revert "http: make HEAD method to work with keep-alive"
f504c9c 
This reverts commit 7afa533.

The change breaks clients like cURL.

Fixes: #38922

PR-URL: #38949
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

mcollina commented Jun 8, 2021

Copy link
Copy Markdown
Member

Landed in f504c9c

mcollina closed this Jun 8, 2021
targos deleted the fix-38922 branch June 8, 2021 16:45
mmarchini mentioned this pull request Jun 9, 2021
Merged
targos added a commit that referenced this pull request Jun 11, 2021
Revert "http: make HEAD method to work with keep-alive"
336571f 
This reverts commit 7afa533.

The change breaks clients like cURL.

Fixes: #38922

PR-URL: #38949
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
targos added a commit that referenced this pull request Jun 11, 2021
Revert "http: make HEAD method to work with keep-alive"
07c55d2 
This reverts commit 7afa533.

The change breaks clients like cURL.

Fixes: #38922

PR-URL: #38949
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
danielleadams mentioned this pull request Jun 14, 2021
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
jasnell jasnell approved these changes
cjihrig cjihrig approved these changes
ronag ronag approved these changes
gengjiawen gengjiawen approved these changes
+5 more reviewers
mmarchini mmarchini approved these changes
mhdawson mhdawson approved these changes
dnlup dnlup approved these changes
bl-ue bl-ue approved these changes
VoltrexKeyva VoltrexKeyva approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Potential breaking change on v14.17.0
Back | FazBrowse Home | New Git URL