Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(52)

Issue 212623010: ICU 52 : update gyp and isolate (Closed)

Created:
6 years, 9 months ago by jungshik at Google
Modified:
6 years, 8 months ago
Reviewers:
Mark Mentovai, Nico
CC:
chromium-reviews
Visibility:
Public.

Description

Update gyp and isolate files for ICU 52.1 Update icu.gyp and related files to list C/C++ source files and header files. Besides, HAVE_DLOPEN is set to 0 explicitly in icu.gyp. It's 0 by default in ICU 4.6.1 but is 1 by default in ICU 52.1, but we don't need it. (it's only necessary for ICU plugins that Chromium does not use). I also applied https://codereview.chromium.org/218153002/ (Nico's CL to simplify icu.gyp). BUG=132145 TEST=None R=mark@chromium.org, thakis@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260941

Patch Set 1 #

Patch Set 2 : gyp file update #

Patch Set 3 : configure.patch added #

Total comments: 1

Patch Set 4 : #

Total comments: 1

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+145 lines, -119 lines) Patch
A + icu52/icu.gyp View 1 2 3 4 21 chunks +67 lines, -75 lines 0 comments Download
A + icu52/icu.gypi View 1 12 chunks +80 lines, -46 lines 0 comments Download
A + icu52/icu.isolate View 1 0 chunks +-1 lines, --1 lines 0 comments Download
A + icu52/icu_nacl.gyp View 1 0 chunks +-1 lines, --1 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
jungshik at Google
Can you take a look? Thanks. BTW, I plan to remove / revise entries in ...
6 years, 9 months ago (2014-03-27 11:08:35 UTC) #1
jungshik at Google
I realize that it's hard to tell what's going on and whether dropping them is ...
6 years, 9 months ago (2014-03-28 11:24:41 UTC) #2
jungshik at Google
https://codereview.chromium.org/212623010/diff/40001/icu52/icu.gyp File icu52/icu.gyp (left): https://codereview.chromium.org/212623010/diff/40001/icu52/icu.gyp#oldcode202 icu52/icu.gyp:202: '-Wno-header-hygiene', This is another slightly non-trivial change. The upstream ...
6 years, 9 months ago (2014-03-28 11:28:36 UTC) #3
jungshik at Google
On 2014/03/28 11:28:36, Jungshik Shin wrote: > https://codereview.chromium.org/212623010/diff/40001/icu52/icu.gyp > File icu52/icu.gyp (left): > > https://codereview.chromium.org/212623010/diff/40001/icu52/icu.gyp#oldcode202 ...
6 years, 8 months ago (2014-04-01 07:30:46 UTC) #4
jungshik at Google
PS 4 has only gyp* files and isolate files.
6 years, 8 months ago (2014-04-01 16:38:46 UTC) #5
Mark Mentovai
LGTM. Would it be sensible to not have the ICU version number be part of ...
6 years, 8 months ago (2014-04-01 17:04:03 UTC) #6
Nico
Thanks for merging in my cl :-) https://codereview.chromium.org/212623010/diff/60001/icu52/icu.gyp File icu52/icu.gyp (right): https://codereview.chromium.org/212623010/diff/60001/icu52/icu.gyp#newcode288 icu52/icu.gyp:288: # v8 ...
6 years, 8 months ago (2014-04-01 17:59:51 UTC) #7
jungshik at Google
On 2014/04/01 17:04:03, Mark Mentovai wrote: > LGTM. Would it be sensible to not have ...
6 years, 8 months ago (2014-04-01 18:22:01 UTC) #8
Mark Mentovai
OK, I didn’t mean for you to change it in this patch. Go with icu52 ...
6 years, 8 months ago (2014-04-01 18:24:16 UTC) #9
jungshik at Google
On 2014/04/01 18:24:16, Mark Mentovai wrote: > OK, I didn’t mean for you to change ...
6 years, 8 months ago (2014-04-01 20:26:22 UTC) #10
Nico
Patch set 5 lgtm. Thanks!
6 years, 8 months ago (2014-04-01 20:31:11 UTC) #11
Mark Mentovai
LGTM too
6 years, 8 months ago (2014-04-01 20:33:08 UTC) #12
jungshik at Google
6 years, 8 months ago (2014-04-01 20:51:49 UTC) #13
Message was sent while issue was closed.
Committed patchset #5 manually as r260941 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698