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

v17.7.1 release proposal #42285

Merged
merged 2 commits into from Mar 10, 2022
Merged

v17.7.1 release proposal #42285

merged 2 commits into from Mar 10, 2022

Conversation

sxa
Copy link
Member

@sxa sxa commented Mar 10, 2022

2022-03-10, Version 17.7.1 (Current), @BethGriggs prepared by @sxa

Notable Changes

Fixed regression in url.resolve()

This release fixes an issue introduced in Node.js v17.7.0 with some URLs that contain @. This issue affected yarn 1. This version reverts the change that introduced the regression.

Commits

  • [96a9e00fb3] - url: revert fix url.parse() for @hostname (Antoine du Hamel) #42280

This reverts commit 010cb71.

Refs: #42279

PR-URL: #42280
Fixes: #42279
Reviewed-By: Stewart X Addison <sxa@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@sxa sxa self-assigned this Mar 10, 2022
@nodejs-github-bot nodejs-github-bot added meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. url Issues and PRs related to the legacy built-in url module. v17.x labels Mar 10, 2022
sxa added a commit that referenced this pull request Mar 10, 2022
Notable changes:

Fixed regression in url.resolve()

This release fixes an issue introduced in Node.js v17.7.0 when the url
has @ characters in it, which showed up when using yarn v1. This
version reverts the change that introduced the regression

PR-URL: #42285
@sxa
Copy link
Member Author

sxa commented Mar 10, 2022

@Trott If you get the chance LMK if you're happy with the wording in the changlog (same as in the description of this PR)

@nodejs-github-bot
Copy link
Collaborator

@aduh95
Copy link
Contributor

aduh95 commented Mar 10, 2022

yarn v1 -> Yarn 1? That's how it's written on https://classic.yarnpkg.com/.

@Trott
Copy link
Member

Trott commented Mar 10, 2022

@Trott If you get the chance LMK if you're happy with the wording in the changlog (same as in the description of this PR)

Looks good to me. I'll submit a suggestion for some typographical changes, but this is fine to land without my suggestions as far as I'm concerned.

Comment on lines 53 to 55
This release fixes an issue introduced in Node.js v17.7.0 when the url
has @ characters in it, which showed up when using yarn v1. This version
reverts the change that introduced the regression.
Copy link
Member

@Trott Trott Mar 10, 2022

Choose a reason for hiding this comment

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

Optional non-blocking suggestion:

Suggested change
This release fixes an issue introduced in Node.js v17.7.0 when the url
has @ characters in it, which showed up when using yarn v1. This version
reverts the change that introduced the regression.
This release fixes an issue introduced in Node.js v17.7.0 with some URLs
that contain `@`. This issue affected yarn v1. This version reverts the
change that introduced the regression.

@bricss
Copy link

bricss commented Mar 10, 2022

Maybe it's possible to pull #42205 into it? 🙄
It has quite nice fixes 🩹

Notable changes:

Fixed regression in url.resolve()

This release fixes an issue introduced in Node.js v17.7.0 with some URLs
that contain `@`. This issue affected yarn 1. This version reverts the
change that introduced the regression.

PR-URL: #42285
@sxa
Copy link
Member Author

sxa commented Mar 10, 2022

Force pushed with both typographical suggestions :-)
Taking out of draft.

@sxa sxa marked this pull request as ready for review March 10, 2022 17:50
@sxa
Copy link
Member Author

sxa commented Mar 10, 2022

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

Re-run at https://ci.nodejs.org/job/node-test-commit/52235

  • SmartOS and OSX failed for some reason in the intiial run, but passed in the re-run
  • ubuntu1804_sharedlibs_debug_x64 failed on a flake only in the initial run
  • Fanned arm builds had one failure in the initial run then the machiens all seemed to be having problems on the re-runm but arm32 passed ok in the armv7l branch of node-test-commit-arm so I'm not concerned that we have a real break

@BethGriggs
Copy link
Member

BethGriggs added a commit that referenced this pull request Mar 10, 2022
@BethGriggs BethGriggs merged commit 8fe7979 into v17.x Mar 10, 2022
BethGriggs pushed a commit that referenced this pull request Mar 10, 2022
Notable changes:

Fixed regression in url.resolve()

This release fixes an issue introduced in Node.js v17.7.0 with some URLs
that contain `@`. This issue affected yarn 1. This version reverts the
change that introduced the regression.

PR-URL: #42285
sxa added a commit to sxa/nodejs.org that referenced this pull request Mar 10, 2022
Refs: nodejs/node#42285

Signed-off-by: Stewart X Addison <sxa@redhat.com>
BethGriggs pushed a commit to nodejs/nodejs.org that referenced this pull request Mar 10, 2022
Refs: nodejs/node#42285

Signed-off-by: Stewart X Addison <sxa@redhat.com>
@BethGriggs BethGriggs deleted the v17.7.1-proposal branch March 10, 2022 20:55
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
Notable changes:

Fixed regression in url.resolve()

This release fixes an issue introduced in Node.js v17.7.0 with some URLs
that contain `@`. This issue affected yarn 1. This version reverts the
change that introduced the regression.

PR-URL: nodejs#42285
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants