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.

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

Revert the change of network interfaces family from string to integer #43054

Closed
wants to merge 5 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented May 11, 2022

This reverts commits 68fb0bf, 4ad342a, and 3a26db9.

Fixes: #43014

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 11, 2022
@aduh95 aduh95 added semver-major PRs that contain breaking changes and should be released in the next major version. tsc-agenda Issues and PRs to discuss during the meetings of the TSC. labels May 11, 2022
@targos
Copy link
Member

targos commented May 11, 2022

IMO the revert doesn't make sense if it is semver-major, because everyone will have to adapt to the change for v18 (which is going to be LTS).

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

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

@mcollina
Copy link
Member

+1 to land it on Node v18.

@mcollina mcollina added semver-major PRs that contain breaking changes and should be released in the next major version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels May 11, 2022
@mcollina
Copy link
Member

mcollina commented May 11, 2022

@nodejs/tsc at Today meeting we did not have quorum to LGTM as a non-semver-major change. Does anybody from the TSC objects to this?

We actually had quorum at the end of the meeting, we approve landing this as semver-minor.

@targos targos added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label May 11, 2022
@mcollina mcollina added semver-minor PRs that contain new features and should be released in the next minor version. dont-land-on-v12.x and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels May 11, 2022
@RaisinTen RaisinTen added the notable-change PRs with changes that should be highlighted in changelogs. label May 11, 2022
lib/dns.js Outdated Show resolved Hide resolved
@RaisinTen
Copy link
Contributor

Is it okay if we remove this from the tsc-agenda now, since everyone in the meeting was +1 with reverting?

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. labels May 11, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 11, 2022
@nodejs-github-bot

This comment was marked as outdated.

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Rubber stamp

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 force-pushed the revert-net-family-integer branch from 070ff18 to 34bb24a Compare June 5, 2022 20:46
@bnoordhuis
Copy link
Member

@aduh95 I remember running into the same issue on smartos a few years ago. I'd be okay with marking the tests flaky.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mhdawson
Copy link
Member

We discussed in the TSC meeting and there were no objections to this landing. Doing that now.

mhdawson pushed a commit that referenced this pull request Jun 15, 2022
Refs: #43014

PR-URL: #43054
Fixes: #43014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@mhdawson
Copy link
Member

Landed as 70b516e

@mhdawson mhdawson closed this Jun 15, 2022
@aduh95 aduh95 mentioned this pull request Jun 15, 2022
danielleadams pushed a commit that referenced this pull request Jun 16, 2022
Refs: #43014

PR-URL: #43054
Fixes: #43014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
danielleadams added a commit that referenced this pull request Jun 16, 2022
Notable changes:

* crypto:
  * remove Node.js-specific webcrypto extensions (Filip Skokan) #43310
  * add CFRG curves to Web Crypto API (Filip Skokan) #42507
* dns:
  * accept `'IPv4'` and `'IPv6'` for `family` (Antoine du Hamel) #43054
* report:
  * add more heap infos in process report (theanarkh) #43116

PR-URL: #43385
danielleadams added a commit that referenced this pull request Jun 16, 2022
Notable changes:

* crypto:
  * remove Node.js-specific webcrypto extensions (Filip Skokan) #43310
  * add CFRG curves to Web Crypto API (Filip Skokan) #42507
* dns:
  * accept `'IPv4'` and `'IPv6'` for `family` (Antoine du Hamel) #43054
* report:
  * add more heap infos in process report (theanarkh) #43116

PR-URL: #43385
danielleadams added a commit that referenced this pull request Jun 16, 2022
Notable changes:

* crypto:
  * remove Node.js-specific webcrypto extensions (Filip Skokan) #43310
  * add CFRG curves to Web Crypto API (Filip Skokan) #42507
* dns:
  * accept `'IPv4'` and `'IPv6'` for `family` (Antoine du Hamel) #43054
* report:
  * add more heap infos in process report (theanarkh) #43116

PR-URL: #43385
@aduh95 aduh95 deleted the revert-net-family-integer branch March 29, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version. tsc-agenda Issues and PRs to discuss during the meetings of the TSC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revert the change of network interfaces family from String to Integer