Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1414)

Issue 314690043: ICU ticket #13053: fix compilation errors and test failures when UCONFIG_NO_BREAK_ITERATION is swit… (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 1 month ago by nrunge
Modified:
6 years, 7 months ago
Base URL:
http://source.icu-project.org/repos/icu/trunk/icu4c
Visibility:
Public.

Description

Make UCONFIG_NO_BREAK_ITERATION work without errors or failures

Patch Set 1 #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -3 lines) Patch
M common/unicode/locdspnm.h View 1 chunk +1 line, -0 lines 2 comments Download
M i18n/unicode/dtfmtsym.h View 1 chunk +1 line, -0 lines 2 comments Download
M test/intltest/strcase.cpp View 7 chunks +15 lines, -2 lines 4 comments Download
M test/intltest/transtst.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4
nrunge
7 years, 1 month ago (2017-03-24 21:17:41 UTC) #1
andy.heninger
LGTM
7 years, 1 month ago (2017-03-24 21:56:15 UTC) #2
markus.icu
https://codereview.appspot.com/314690043/diff/1/common/unicode/locdspnm.h File common/unicode/locdspnm.h (right): https://codereview.appspot.com/314690043/diff/1/common/unicode/locdspnm.h#newcode13 common/unicode/locdspnm.h:13: #include "unicode/strenum.h" Please move this to the other group ...
7 years, 1 month ago (2017-03-24 23:00:05 UTC) #3
nrunge
7 years, 1 month ago (2017-03-27 17:42:29 UTC) #4
Done.

https://codereview.appspot.com/314690043/diff/1/common/unicode/locdspnm.h
File common/unicode/locdspnm.h (right):

https://codereview.appspot.com/314690043/diff/1/common/unicode/locdspnm.h#new...
common/unicode/locdspnm.h:13: #include "unicode/strenum.h"
On 2017/03/24 23:00:04, markus.icu wrote:
> Please move this to the other group of #includes.
> 
> We include utypes.h first to get the config switches.

Done.

https://codereview.appspot.com/314690043/diff/1/i18n/unicode/dtfmtsym.h
File i18n/unicode/dtfmtsym.h (right):

https://codereview.appspot.com/314690043/diff/1/i18n/unicode/dtfmtsym.h#newco...
i18n/unicode/dtfmtsym.h:23: #include "unicode/strenum.h"
On 2017/03/24 23:00:05, markus.icu wrote:
> ditto

Done.

https://codereview.appspot.com/314690043/diff/1/test/intltest/strcase.cpp
File test/intltest/strcase.cpp (right):

https://codereview.appspot.com/314690043/diff/1/test/intltest/strcase.cpp#new...
test/intltest/strcase.cpp:782: int32_t destLength = 0;
On 2017/03/24 23:00:05, markus.icu wrote:
> you should not need the = 0 initializer: the variable is not read until
written,
> even after taking out the following few lines

Done.

https://codereview.appspot.com/314690043/diff/1/test/intltest/strcase.cpp#new...
test/intltest/strcase.cpp:827: int32_t result = 0;
On 2017/03/24 23:00:05, markus.icu wrote:
> ditto

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b