GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — url: revert "validate ipv4 part length" by aduh95 · Pull Request #42940 · nodejs/node

Conversation

@aduh95

@aduh95 aduh95 commented May 2, 2022

Copy link
Copy Markdown
Contributor

This reverts commit 87d0d7a.

Refs: #42915 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@aduh95
url: revert "validate ipv4 part length"
05dcc02 
This reverts commit 87d0d7a.

Refs: nodejs#42915
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels May 2, 2022
targos
targos approved these changes May 2, 2022
View reviewed changes

@targos targos left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Don't we usually keep the revert message created by Git?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@aduh95

aduh95 commented May 2, 2022

Copy link
Copy Markdown
Contributor Author

Don't we usually keep the revert message created by Git?

Looking at git log --grep='This reverts commit', there are two schools of thought on the question. Looking at our commit message guidelines, it doesn't say revert commits should be treated differently, so I'm going to leave the message as is, whoever lands this can choose a different commit message.
targos reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95
fixup! url: revert "validate ipv4 part length"
7352d8b
VoltrexKeyva
VoltrexKeyva approved these changes May 2, 2022
View reviewed changes
@targos targos added the dont-land-on-v18.x label May 2, 2022
@addaleax

addaleax commented May 2, 2022

Copy link
Copy Markdown
Member

Looking at our commit message guidelines, it doesn't say revert commits should be treated differently

Our release tooling does have special treatment for the standard git revert message format. I’d rather edit our commit message guidelines to reflect that than ignore it 🙂 e.g. https://github.com/nodejs/node/blob/1d8a320a04d75b2143be0bc97b10dfcd9e64defa/doc/changelogs/CHANGELOG_V16.md#commits-3 contains

  • [6af3e34b6b] - Revert "async_hooks: merge resource_symbol with owner_symbol" (Darshan Sen) #40741
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Trott
Trott approved these changes May 2, 2022
View reviewed changes
@sepehrst

sepehrst commented May 2, 2022

Copy link
Copy Markdown

Since this is a revert, is it reasonable to fast-track so it can be included in the 3rd May builds?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95

aduh95 commented May 2, 2022

Copy link
Copy Markdown
Contributor Author

dont-land-on-v18.x

This PR and the on it reverts has the https://github.com/nodejs/node/labels/dont-land-on-v18.x, so none will be included on May 3rd release anyway :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs nodejs deleted a comment from amusaber950 May 2, 2022
@nodejs nodejs deleted a comment from amusaber950 May 2, 2022
@richardlau richardlau added dont-land-on-v14.x labels May 3, 2022
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels May 4, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label May 4, 2022
@nodejs-github-bot

nodejs-github-bot commented May 4, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43881/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 5, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43885/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95 aduh95 added the commit-queue PRs queued for automated landing through the Commit Queue. label May 5, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label May 5, 2022
@nodejs-github-bot
nodejs-github-bot merged commit a3ff4bf into nodejs:master May 5, 2022
@nodejs-github-bot

nodejs-github-bot commented May 5, 2022

Copy link
Copy Markdown
Collaborator

Landed in a3ff4bf
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95
aduh95 deleted the revert-url-ipv4-fix branch May 5, 2022 11:49
This was referenced May 12, 2022
CI Reliability 2022-05-12 nodejs/reliability#273
Open
CI Reliability 2022-05-12 nodejs/reliability#275
Closed
CI Reliability 2022-05-13 nodejs/reliability#276
Open
@F3n67u F3n67u mentioned this pull request Jun 4, 2022
test, url: update WHATWG URL parser to align with latest spec #43190
Merged
danielleadams pushed a commit that referenced this pull request Jun 11, 2022
@aduh95 @danielleadams
url: revert "validate ipv4 part length"
8fc42d3 
This reverts commit 87d0d7a.

Refs: #42915

PR-URL: #42940
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request Jun 11, 2022
v18.4.0 proposal #43385
Merged
@RafaelGSS RafaelGSS mentioned this pull request Oct 14, 2022
2022-10-18, Version 19.0.0 (Current) #44626
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes
+2 more reviewers
@targos targos targos approved these changes
@VoltrexKeyva VoltrexKeyva VoltrexKeyva approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@aduh95 @addaleax @sepehrst @nodejs-github-bot @Trott @targos @VoltrexKeyva @richardlau
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.