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

Issue 323340043: ticket:13285 Adding NumberingSystem constructor methods to DecimalFormatSymbols (J and C). (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 4 weeks ago by sffc
Modified:
2 months, 1 week ago
Reviewers:
markus.icu
Base URL:
svn+icussh://source.icu-project.org/repos/icu/trunk/
Visibility:
Public.

Description

ticket:13285 Adding NumberingSystem constructor methods to DecimalFormatSymbols (J and C).

Patch Set 1 #

Total comments: 3

Patch Set 2 : Prelimiary feedback from Markus #

Total comments: 23

Patch Set 3 : Second round of feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+181 lines, -17 lines) Patch
M icu4c/source/i18n/dcfmtsym.cpp View 1 2 4 chunks +17 lines, -11 lines 0 comments Download
M icu4c/source/i18n/unicode/dcfmtsym.h View 1 2 3 chunks +23 lines, -1 line 0 comments Download
M icu4c/source/test/intltest/tsdcfmsy.h View 1 chunk +1 line, -0 lines 0 comments Download
M icu4c/source/test/intltest/tsdcfmsy.cpp View 1 2 2 chunks +44 lines, -0 lines 0 comments Download
M icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java View 1 2 5 chunks +59 lines, -5 lines 0 comments Download
M icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/IntlTestDecimalFormatSymbols.java View 1 2 2 chunks +37 lines, -0 lines 0 comments Download

Messages

Total messages: 10
sffc
I have the API proposal ready to send if the change looks good.
2 months, 4 weeks ago (2017-07-27 03:11:49 UTC) #1
sffc
https://codereview.appspot.com/323340043/diff/1/icu4c/source/test/intltest/tsdcfmsy.cpp File icu4c/source/test/intltest/tsdcfmsy.cpp (right): https://codereview.appspot.com/323340043/diff/1/icu4c/source/test/intltest/tsdcfmsy.cpp#newcode262 icu4c/source/test/intltest/tsdcfmsy.cpp:262: {"my", "mymr", "၁,၂၃၄.၅၆", "၊"}, This construct gives the following ...
2 months, 4 weeks ago (2017-07-27 06:15:59 UTC) #2
markus.icu
API LGTM Didn't review impl or test. https://codereview.appspot.com/323340043/diff/1/icu4c/source/test/intltest/tsdcfmsy.cpp File icu4c/source/test/intltest/tsdcfmsy.cpp (right): https://codereview.appspot.com/323340043/diff/1/icu4c/source/test/intltest/tsdcfmsy.cpp#newcode262 icu4c/source/test/intltest/tsdcfmsy.cpp:262: {"my", "mymr", ...
2 months, 4 weeks ago (2017-07-27 20:52:17 UTC) #3
sffc
Prelimiary feedback from Markus
2 months, 4 weeks ago (2017-07-27 21:14:45 UTC) #4
sffc
The API proposal has been sent. https://codereview.appspot.com/323340043/diff/1/icu4c/source/test/intltest/tsdcfmsy.cpp File icu4c/source/test/intltest/tsdcfmsy.cpp (right): https://codereview.appspot.com/323340043/diff/1/icu4c/source/test/intltest/tsdcfmsy.cpp#newcode262 icu4c/source/test/intltest/tsdcfmsy.cpp:262: {"my", "mymr", "၁,၂၃၄.၅၆", ...
2 months, 4 weeks ago (2017-07-27 21:15:21 UTC) #5
markus.icu
https://codereview.appspot.com/323340043/diff/20001/icu4c/source/i18n/dcfmtsym.cpp File icu4c/source/i18n/dcfmtsym.cpp (right): https://codereview.appspot.com/323340043/diff/20001/icu4c/source/i18n/dcfmtsym.cpp#newcode117 icu4c/source/i18n/dcfmtsym.cpp:117: : UObject(), funky indentation Not quite Google style, but ...
2 months, 2 weeks ago (2017-08-08 23:44:38 UTC) #6
sffc
Second round of feedback
2 months, 1 week ago (2017-08-17 02:47:02 UTC) #7
sffc
https://codereview.appspot.com/323340043/diff/20001/icu4c/source/i18n/dcfmtsym.cpp File icu4c/source/i18n/dcfmtsym.cpp (right): https://codereview.appspot.com/323340043/diff/20001/icu4c/source/i18n/dcfmtsym.cpp#newcode117 icu4c/source/i18n/dcfmtsym.cpp:117: : UObject(), On 2017/08/08 23:44:37, markus.icu wrote: > funky ...
2 months, 1 week ago (2017-08-17 02:47:53 UTC) #8
markus.icu
LGTM
2 months, 1 week ago (2017-08-17 21:54:00 UTC) #9
sffc
2 months, 1 week ago (2017-08-17 23:49:17 UTC) #10
On 2017/08/17 21:54:00, markus.icu wrote:
> LGTM

Committed revision 40345.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted