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

Issue 1052333003: Remove additional calendars in the ICU data (Closed)

Created:
5 years, 8 months ago by jungshik at Google
Modified:
5 years, 8 months ago
Reviewers:
Mark Mentovai
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/deps/icu.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Remove additional calendars in the ICU data Due to a bug in patch_locale.sh, calendars intended for being stripped away are not removed from the ICU data for Android. Fixing the bug cuts down the ICU data size by 10kB on Android. BUG=472142 TEST=android/icudtl.dat is ~10kB smaller than before. (6,267,808 bytes) R=mark@chromium.org Committed: https://chromium.googlesource.com/chromium/deps/icu/+/e4c31439828d356525b71ef81a6d61ea50d7d673

Patch Set 1 #

Total comments: 1

Patch Set 2 : use parameter expansion #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -2 lines) Patch
M android/icudtl.dat View Binary file 0 comments Download
M android/patch_locale.sh View 1 1 chunk +5 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
jungshik at Google
Meant to do this along with the previous CL, but forgot. PTAL. Thanks.
5 years, 8 months ago (2015-04-02 23:01:05 UTC) #2
Mark Mentovai
LGTM https://codereview.chromium.org/1052333003/diff/1/android/patch_locale.sh File android/patch_locale.sh (right): https://codereview.chromium.org/1052333003/diff/1/android/patch_locale.sh#newcode175 android/patch_locale.sh:175: CAL_PATTERN="(${COMMON_CALENDARS})" You can write all of this in ...
5 years, 8 months ago (2015-04-03 12:51:09 UTC) #3
jungshik at Google
Committed patchset #2 (id:10001) manually as e4c31439828d356525b71ef81a6d61ea50d7d673 (presubmit successful).
5 years, 8 months ago (2015-04-03 17:10:30 UTC) #4
jungshik at Google
5 years, 8 months ago (2015-04-03 17:11:01 UTC) #5
Message was sent while issue was closed.
On 2015/04/03 12:51:09, Mark Mentovai wrote:
> LGTM
> 
> https://codereview.chromium.org/1052333003/diff/1/android/patch_locale.sh
> File android/patch_locale.sh (right):
> 
>
https://codereview.chromium.org/1052333003/diff/1/android/patch_locale.sh#new...
> android/patch_locale.sh:175: CAL_PATTERN="(${COMMON_CALENDARS})"
> You can write all of this in one line:
> 
>   CAL_PATTERN="${COMMON_CALENDARS}${EXTRA_CAL:+|${EXTRA_CAL}}"
> 
> ${var:+text} expands to nothing if var is empty and to text if var is
nonempty.

Thanks. I switched to a param expansion as suggested.

Powered by Google App Engine
This is Rietveld 408576698