Skip to content

deps: ICU 59.1 bump#12486

Closed
srl295 wants to merge 0 commit intonodejs:masterfrom
srl295:icu4c-59.1
Closed

deps: ICU 59.1 bump#12486
srl295 wants to merge 0 commit intonodejs:masterfrom
srl295:icu4c-59.1

Conversation

@srl295
Copy link
Member

@srl295 srl295 commented Apr 18, 2017 •
edited
Loading

Fixes: #12077

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

deps

@srl295 srl295 self-assigned this Apr 18, 2017
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Apr 18, 2017
@srl295 srl295 added the wip Issues and PRs that are still a work in progress. label Apr 18, 2017
@srl295 srl295 mentioned this pull request Apr 18, 2017
Closed
@refack refack added i18n-api Issues and PRs related to the i18n implementation. v8 engine Issues and PRs related to the V8 dependency. labels Apr 18, 2017
@srl295
Copy link
Member Author

srl295 commented May 8, 2017

OK! v8 now fixed, this should be good. #11753 (comment)

@srl295
Copy link
Member Author

srl295 commented May 8, 2017

^ rebased, PTAL all. Will start CI.

⚠️ this may have porting/compiler hurdles ahead (C++11 etc)

@srl295 srl295 requested review from MylesBorins, jasnell and refack May 8, 2017 19:31
@srl295
Copy link
Member Author

srl295 commented May 8, 2017

fwiw code starting with deps/icu-small/source/common/appendable.cpp in the diffs should all be ICU code under deps.

@srl295
Copy link
Member Author

srl295 commented May 8, 2017

^ CI green ✅

refack
refack approved these changes May 9, 2017
View reviewed changes
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

Looks good. Works on my machine.

@MylesBorins
Copy link
Contributor

MylesBorins commented May 9, 2017

V8 Ci: https://ci.nodejs.org/job/node-test-commit-v8-linux/694/

@srl295
Copy link
Member Author

srl295 commented May 9, 2017

Thanks! FYI @ilovezfs

mhdawson
mhdawson approved these changes May 9, 2017
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

Rubber Stamp LGTM.

@mhdawson
Copy link
Member

mhdawson commented May 9, 2017

@srl295 could you paste the link to the CI run into one of the comments for posterity.

@srl295
Copy link
Member Author

srl295 commented May 9, 2017

@mhdawson https://ci.nodejs.org/job/node-test-pull-request/7951/ ?

@srl295 srl295 closed this May 9, 2017
srl295 added a commit to srl295/node that referenced this pull request May 9, 2017
@srl295
deps: ICU 59.1 bump
5d0a770 
* No feature changes.
* Bug fixes.
* Details: http://site.icu-project.org/download/59

Fixes: nodejs#12077
PR-URL: nodejs#12486
Refs: nodejs#7844
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@srl295 srl295 deleted the icu4c-59.1 branch May 9, 2017 22:23
@srl295 srl295 mentioned this pull request May 9, 2017
Closed
@ArchangeGabriel ArchangeGabriel mentioned this pull request May 14, 2017
Closed
anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
@srl295
deps: ICU 59.1 bump
0286097 
* No feature changes.
* Bug fixes.
* Details: http://site.icu-project.org/download/59

Fixes: nodejs#12077
PR-URL: nodejs#12486
Refs: nodejs#7844
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@jasnell jasnell mentioned this pull request May 28, 2017
Closed
@srl295 srl295 mentioned this pull request May 30, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
@gibfahn
Copy link
Member

gibfahn commented Jun 20, 2017

I'm assuming this shouldn't land on v6.x as it depends on V8 5.9. LMK if wrong.

@srl295
Copy link
Member Author

srl295 commented Aug 30, 2017

@gibfahn yes, it would need v8 backported also.

@gibfahn
Copy link
Member

gibfahn commented Aug 31, 2017

@srl295 yep, I've confirmed it went into 8.0.0 so we're good there.

@srl295 srl295 mentioned this pull request Nov 10, 2017
Closed
srl295 added a commit to srl295/node that referenced this pull request Jan 18, 2018
@srl295
deps: ICU 59.1 bump
9f571ef 
* No feature changes.
* Bug fixes.
* Details: http://site.icu-project.org/download/59

Fixes: nodejs#12077
PR-URL: nodejs#12486
Refs: nodejs#7844
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@srl295 srl295 mentioned this pull request Jan 18, 2018
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@mhdawson mhdawson mhdawson approved these changes

@MylesBorins MylesBorins Awaiting requested review from MylesBorins

+2 more reviewers

@refack refack refack approved these changes

@gibfahn gibfahn gibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@srl295 srl295

Labels

build Issues and PRs related to build files or the CI. i18n-api Issues and PRs related to the i18n implementation. v8 engine Issues and PRs related to the V8 dependency. wip Issues and PRs that are still a work in progress.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

deps: ICU 59 bump

7 participants

@srl295 @MylesBorins @mhdawson @gibfahn @refack @jasnell @nodejs-github-bot