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

dns: remove dns.promises experimental warning #26592

Merged
merged 3 commits into from
Mar 29, 2019
Merged

dns: remove dns.promises experimental warning #26592

merged 3 commits into from
Mar 29, 2019

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 11, 2019

In a similar spirit to #26581.

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

@nodejs-github-bot
Copy link
Collaborator

@cjihrig build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2864/pipeline

@nodejs-github-bot nodejs-github-bot added the dns Issues and PRs related to the dns subsystem. label Mar 11, 2019
@devsnek
Copy link
Member

devsnek commented Mar 11, 2019

i have a similar concern here.

as long as x.promises existing doesn't block x/promises from existing this is probably fine.

@vsemozhetbyt vsemozhetbyt mentioned this pull request Mar 11, 2019
Closed
6 tasks
@Trott
Copy link
Member

Trott commented Mar 11, 2019

Same question as in the other PR: This leaves it as experimental in the docs. That's intentional? (I'd prefer we make it stable in the docs and label this semver-major but that's just me!)

@jasnell
Copy link
Member

jasnell commented Mar 11, 2019

Perhaps it's time just to take it out of experimental?

@targos
Copy link
Member

targos commented Mar 11, 2019

Did we get feedback or reported usage ?

addaleax
addaleax approved these changes Mar 12, 2019
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Like @jdalton suggested in the other PR, it probably makes sense to make this enumerable as well

@BridgeAR
Copy link
Member

Ping @cjihrig

@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 27, 2019

as long as x.promises existing doesn't block x/promises from existing this is probably fine.

It shouldn't block it.

This leaves it as experimental in the docs.

I've add a semver major commit to address it.

Perhaps it's time just to take it out of experimental?

Agreed. It's been ~9 months with no significant issues.

Did we get feedback or reported usage ?

Not that I'm aware of.

it probably makes sense to make this enumerable as well

Done.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 27, 2019 •
edited by cjihrig
Loading

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

EDIT(cjihrig): CI was yellow.

@cjihrig cjihrig added the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 28, 2019
@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 29, 2019

This requires @nodejs/tsc review because it's semver major. (@addaleax you may want to re-review).

mcollina
mcollina approved these changes Mar 29, 2019
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

@cjihrig
dns: remove dns.promises experimental warning
aa2ac19 
PR-URL: nodejs#26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@cjihrig
dns: make dns.promises enumerable
61e4d89 
PR-URL: nodejs#26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@cjihrig
doc: move dns.promises to stable status
d8c2803 
PR-URL: nodejs#26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 29, 2019

Landed in 415a825...d8c2803. Thanks for the reviews.

@cjihrig cjihrig closed this Mar 29, 2019
@cjihrig cjihrig deleted the dns-promises branch March 29, 2019 22:58
@cjihrig cjihrig merged commit d8c2803 into nodejs:master Mar 29, 2019
@addaleax addaleax mentioned this pull request Mar 30, 2019
Closed
2 tasks
@BethGriggs BethGriggs mentioned this pull request Apr 2, 2019
Closed
@addaleax
Copy link
Member

addaleax commented Apr 2, 2019

@cjihrig Sorry, I missed that you had labelled this semver-major. Is there any particular reason? I’d prefer to remove the label.

@cjihrig
Copy link
Contributor Author

cjihrig commented Apr 2, 2019

@addaleax because of the change in enumerability. I was being overly cautious, and would also be fine dropping the label.

@addaleax addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 2, 2019
@addaleax addaleax removed the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 2, 2019
BethGriggs pushed a commit that referenced this pull request Apr 5, 2019
@cjihrig @BethGriggs
dns: remove dns.promises experimental warning
f3d890f 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 5, 2019
@cjihrig @BethGriggs
dns: make dns.promises enumerable
e58f4eb 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 5, 2019
@cjihrig @BethGriggs
doc: move dns.promises to stable status
0d8d842 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 8, 2019
@cjihrig @BethGriggs
dns: remove dns.promises experimental warning
d3c1de3 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Signed-off-by: Beth Griggs <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 8, 2019
@cjihrig @BethGriggs
dns: make dns.promises enumerable
ac12109 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Signed-off-by: Beth Griggs <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 8, 2019
@cjihrig @BethGriggs
doc: move dns.promises to stable status
4a7a84a 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Signed-off-by: Beth Griggs <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Apr 9, 2019
Merged
BethGriggs added a commit that referenced this pull request Apr 10, 2019
@BethGriggs
2019-04-10, Version 11.14.0 (Current)
ebfb67a 
Notable changes:

- child_process: doc deprecate ChildProcess.\_channel (cjihrig)
  [#26982](#26982)
- deps: update nghttp2 to 1.37.0 (gengjiawen)
  [#26990](#26990)
- dns:
  - make dns.promises enumerable (cjihrig)
    [#26592](#26592)
  - remove dns.promises experimental warning (cjihrig)
    [#26592](#26592)
- stream: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)
- worker: use copy of process.env (Anna Henningsen)
  [#26544](#26544)

PR-URL: #27163
BethGriggs added a commit that referenced this pull request Apr 11, 2019
@BethGriggs
2019-04-11, Version 11.14.0 (Current)
cd026f8 
Notable changes:

- child_process: doc deprecate ChildProcess.\_channel (cjihrig)
  [#26982](#26982)
- deps: update nghttp2 to 1.37.0 (gengjiawen)
  [#26990](#26990)
- dns:
  - make dns.promises enumerable (cjihrig)
    [#26592](#26592)
  - remove dns.promises experimental warning (cjihrig)
    [#26592](#26592)
- fs: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581] (#26581)
- stream: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)
- worker: use copy of process.env (Anna Henningsen)
  [#26544](#26544)

PR-URL: #27163
BethGriggs added a commit that referenced this pull request Apr 11, 2019
@BethGriggs
2019-04-11, Version 11.14.0 (Current)
d424560 
Notable changes:

- child_process: doc deprecate ChildProcess.\_channel (cjihrig)
  [#26982](#26982)
- deps: update nghttp2 to 1.37.0 (gengjiawen)
  [#26990](#26990)
- dns:
  - make dns.promises enumerable (cjihrig)
    [#26592](#26592)
  - remove dns.promises experimental warning (cjihrig)
    [#26592](#26592)
- fs: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581] (#26581)
- stream: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)
- worker: use copy of process.env (Anna Henningsen)
  [#26544](#26544)

PR-URL: #27163
BethGriggs pushed a commit that referenced this pull request Oct 18, 2019
@cjihrig @BethGriggs
dns: remove dns.promises experimental warning
fa27aac 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Oct 18, 2019
@cjihrig @BethGriggs
dns: make dns.promises enumerable
fcc22d3 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Oct 18, 2019
@cjihrig @BethGriggs
doc: move dns.promises to stable status
90fb146 
PR-URL: #26592
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs added a commit that referenced this pull request Oct 18, 2019
@BethGriggs
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
44ef30e 
Notable changes:

- **deps**: upgrade openssl sources to 1.1.1d (Sam Roberts)
  [#29921](#29921)
- **dns**: remove dns.promises experimental warning (cjihrig)
  [#26592](#26592)
- **fs**: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581](#26581)
- **n-api**: mark version 5 N-APIs as stable (Gabriel Schulhof)
  [#29401](#29401)
- **stream**: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)

PR-URL: #29875
@BethGriggs BethGriggs mentioned this pull request Oct 18, 2019
Merged
BethGriggs added a commit that referenced this pull request Oct 19, 2019
@BethGriggs
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
c3e596f 
Notable changes:

- **deps**: update npm to 6.11.3 (claudiahdz)
  [#29430](#29430)
- **deps**: upgrade openssl sources to 1.1.1d (Sam Roberts)
  [#29921](#29921)
- **dns**: remove dns.promises experimental warning (cjihrig)
  [#26592](#26592)
- **fs**: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581](#26581)
- **n-api**: mark version 5 N-APIs as stable (Gabriel Schulhof)
  [#29401](#29401)
- **stream**: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)

PR-URL: #29875
BethGriggs added a commit that referenced this pull request Oct 21, 2019
@BethGriggs
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
ae884d3 
Notable changes:

- **deps**: update npm to 6.11.3 (claudiahdz)
  [#29430](#29430)
- **deps**: upgrade openssl sources to 1.1.1d (Sam Roberts)
  [#29921](#29921)
- **dns**: remove dns.promises experimental warning (cjihrig)
  [#26592](#26592)
- **fs**: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581](#26581)
- **n-api**: mark version 5 N-APIs as stable (Gabriel Schulhof)
  [#29401](#29401)
- **stream**: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)

PR-URL: #29875
BethGriggs added a commit that referenced this pull request Oct 22, 2019
@BethGriggs
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
5dae3ef 
Notable changes:

* crypto:
  * add support for chacha20-poly1305 for AEAD (chux0519)
    #24081
  * increase maxmem range from 32 to 53 bits (Tobias Nießen)
    #28799
* deps:
  * update npm to 6.11.3 (claudiahdz)
    #29430
  * upgrade openssl sources to 1.1.1d (Sam Roberts)
    #29921
* dns:
  * remove dns.promises experimental warning (cjihrig)
    #26592
* fs:
  * remove experimental warning for fs.promises (Anna Henningsen)
    #26581
* http:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* http2:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* n-api:
  * make func argument of napi\_create\_threadsafe\_function optional
    (legendecas)
    #27791
  * mark version 5 N-APIs as stable (Gabriel Schulhof)
    #29401
  * implement date object (Jarrod Connolly)
    #25917
* process:
  * add --unhandled-rejections flag (Ruben Bridgewater)
    #26599
* stream:
  * implement Readable.from async iterator utility (Guy Bedford)
    #27660
  * make Symbol.asyncIterator support stable (Matteo Collina)
    #26989

PR-URL: #29875
BethGriggs added a commit that referenced this pull request Oct 22, 2019
@BethGriggs
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
afbbcb0 
Notable changes:

* crypto:
  * add support for chacha20-poly1305 for AEAD (chux0519)
    #24081
  * increase maxmem range from 32 to 53 bits (Tobias Nießen)
    #28799
* deps:
  * update npm to 6.11.3 (claudiahdz)
    #29430
  * upgrade openssl sources to 1.1.1d (Sam Roberts)
    #29921
* dns:
  * remove dns.promises experimental warning (cjihrig)
    #26592
* fs:
  * remove experimental warning for fs.promises (Anna Henningsen)
    #26581
* http:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* http2:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* n-api:
  * make func argument of napi\_create\_threadsafe\_function optional
    (legendecas)
    #27791
  * mark version 5 N-APIs as stable (Gabriel Schulhof)
    #29401
  * implement date object (Jarrod Connolly)
    #25917
* process:
  * add --unhandled-rejections flag (Ruben Bridgewater)
    #26599
* stream:
  * implement Readable.from async iterator utility (Guy Bedford)
    #27660
  * make Symbol.asyncIterator support stable (Matteo Collina)
    #26989

PR-URL: #29875
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

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
dns Issues and PRs related to the dns subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@cjihrig @nodejs-github-bot @devsnek @Trott @jasnell @targos @BridgeAR @addaleax @mcollina @BethGriggs