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

V12.x icu 69 support. #39451

Closed
wants to merge 2 commits into from
Closed

V12.x icu 69 support. #39451

wants to merge 2 commits into from

Conversation

jameshilliard
Copy link

This is #38497 rebased on v12.x-staging.

@github-actions github-actions bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v12.x v8 engine Issues and PRs related to the V8 dependency. labels Jul 19, 2021
@jameshilliard jameshilliard mentioned this pull request Jul 19, 2021
Closed
targos and others added 2 commits July 23, 2021 12:23
@targos @richardlau
deps: V8: cherry-pick 035c305ce776
af1a3fd 
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: nodejs#38497
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
@richardlau
deps: restore minimum ICU version to 65
7d8c7a4 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: nodejs#39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 23, 2021 •
edited by richardlau
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/39236/ (✔️)
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/4171/

@richardlau
Copy link
Member

Landed in 8a9f949...632a3ae.

@richardlau richardlau closed this Jul 23, 2021
richardlau pushed a commit that referenced this pull request Jul 23, 2021
@targos @richardlau
deps: V8: cherry-pick 035c305ce776
7407caa 
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Backport-PR-URL: #39451
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
richardlau added a commit that referenced this pull request Jul 23, 2021
@richardlau
deps: restore minimum ICU version to 65
632a3ae 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
@jameshilliard jameshilliard deleted the v12.x-icu branch July 23, 2021 15:48
guybedford pushed a commit that referenced this pull request Jul 26, 2021
@targos @guybedford
deps: V8: cherry-pick 035c305ce776
99f3b73 
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Backport-PR-URL: #39451
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
guybedford pushed a commit that referenced this pull request Jul 26, 2021
@richardlau @guybedford
deps: restore minimum ICU version to 65
1b7af5b 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau pushed a commit that referenced this pull request Jul 26, 2021
@targos @richardlau
deps: V8: cherry-pick 035c305ce776
e459c79 
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Backport-PR-URL: #39451
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
richardlau added a commit that referenced this pull request Jul 26, 2021
@richardlau
deps: restore minimum ICU version to 65
39e9cd5 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BethGriggs BethGriggs BethGriggs approved these changes

@richardlau richardlau richardlau approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jameshilliard @nodejs-github-bot @richardlau @BethGriggs @targos