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

util: use Set to store deprecation codes #28113

Closed
wants to merge 1 commit into from

Conversation

dnalborczyk
Copy link
Contributor

uses Set over Object to store deprecation codes.

  • 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

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Jun 6, 2019
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.

I’m assuming this doesn’t have any impact on snapshotting but /cc @joyeecheung just to be sure.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Jun 6, 2019

@addaleax the same file already includes a Set

const experimentalWarnings = new Set();

Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

Thanks for the ping @addaleax . This does not really affect snapshotting - we have more places that use Map and Set during bootstrap, adding one more does not really matter since the correct solution is to implement rehashing for those instead :)

@nodejs-github-bot
Copy link
Collaborator

@ZYSzys ZYSzys added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 8, 2019
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Trott pushed a commit to Trott/io.js that referenced this pull request Jun 9, 2019
PR-URL: nodejs#28113
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott
Copy link
Member

Trott commented Jun 9, 2019

Landed in 0c1fd20

@Trott Trott closed this Jun 9, 2019
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
PR-URL: #28113
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
@dnalborczyk dnalborczyk deleted the codes-warned branch June 19, 2019 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants