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

lib: update punycode to 2.1.1 #21768

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Jul 11, 2018

Blocked to give deference to a potential pull request resulting from #21761. If the OP there doesn't open something in a few days, this can land.

I'm not concerned about the var/let in loop stuff. This is legacy/deprecated. But just to be on the safe side, I'll add labels to indicate this should not land on 6.x , where the version of V8 is such that the whole let-in-loops thing matters.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@Trott Trott added blocked PRs that are blocked by other issues or PRs. dont-land-on-v6.x punycode Issues and PRs related to the punycode module bundled in Node.js. labels Jul 11, 2018
@@ -419,7 +419,7 @@ const punycode = {
* @memberOf punycode
* @type String
*/
'version': '2.0.0',
'version': '2.1.0',
Copy link
Contributor

@mscdex mscdex Jul 11, 2018

Choose a reason for hiding this comment

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

Is this correct? The PR title says this is an upgrade to 2.1.1?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did npm install punycode@2.1.1 so...uh...mistake in the punycode release? /cc @mathiasbynens

Copy link
Member

Choose a reason for hiding this comment

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

That was missed in the 2.1.1 release. The code did not change in that release so it should be fine to land this as is.

@BridgeAR
Copy link
Member

@Trott let / var can also be an issue on 8 but that is not likely anymore, especially in these cases.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 12, 2018
@Trott Trott removed the blocked PRs that are blocked by other issues or PRs. label Jul 15, 2018
@Trott
Copy link
Member Author

Trott commented Jul 15, 2018

Unblocking. PTAL

@Trott
Copy link
Member Author

Trott commented Jul 15, 2018

Trott added a commit to Trott/io.js that referenced this pull request Jul 16, 2018
PR-URL: nodejs#21768
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@Trott
Copy link
Member Author

Trott commented Jul 16, 2018

Landed in ab10bfe

@Trott Trott closed this Jul 16, 2018
targos pushed a commit that referenced this pull request Jul 17, 2018
PR-URL: #21768
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@targos targos mentioned this pull request Jul 17, 2018
@Trott Trott deleted the punycode-2.1.1 branch January 13, 2022 22:49
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. punycode Issues and PRs related to the punycode module bundled in Node.js.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants