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

Issue 5627046: CPP: Fix PhoneNumberMatcher build in Chrome. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 2 months ago by philippe
Modified:
2 years, 2 months ago
CC:
shaopengjia, georgey1
Base URL:
https://libphonenumber.googlecode.com/svn/trunk
Visibility:
Public.

Description

CPP: Fix PhoneNumberMatcher build in Chrome.

Contributors:
  - leandrogracia@chromium.org
  - georgey@chromium.org

Committed: https://code.google.com/p/libphonenumber/source/detail?r=424

Patch Set 1 #

Total comments: 2

Patch Set 2 : Update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -4 lines) Patch
M cpp/src/phonenumbers/phonenumbermatcher.cc View 1 chunk +1 line, -1 line 0 comments Download
M cpp/test/phonenumbers/phonenumbermatcher_test.cc View 1 1 chunk +4 lines, -3 lines 0 comments Download

Messages

Total messages: 5
philippe
2 years, 2 months ago #1
leandrogracia
Just one minor nit. Thanks for fixing this. http://codereview.appspot.com/5627046/diff/1/cpp/test/phonenumbers/phonenumbermatcher_test.cc File cpp/test/phonenumbers/phonenumbermatcher_test.cc (right): http://codereview.appspot.com/5627046/diff/1/cpp/test/phonenumbers/phonenumbermatcher_test.cc#newcode531 cpp/test/phonenumbers/phonenumbermatcher_test.cc:531: EXPECT_TRUE(IsLatinLetter(UnicodeString("\xCC\x81")[0])); ...
2 years, 2 months ago #2
lararennie
LGTM
2 years, 2 months ago #3
philippe
http://codereview.appspot.com/5627046/diff/1/cpp/test/phonenumbers/phonenumbermatcher_test.cc File cpp/test/phonenumbers/phonenumbermatcher_test.cc (right): http://codereview.appspot.com/5627046/diff/1/cpp/test/phonenumbers/phonenumbermatcher_test.cc#newcode531 cpp/test/phonenumbers/phonenumbermatcher_test.cc:531: EXPECT_TRUE(IsLatinLetter(UnicodeString("\xCC\x81")[0])); On 2012/02/04 23:56:44, leandrogracia wrote: > fromUTF8 is ...
2 years, 2 months ago #4
leandrogracia
2 years, 2 months ago #5
LGTM. Thanks.

On 2012/02/06 17:12:32, philippe wrote:
>
http://codereview.appspot.com/5627046/diff/1/cpp/test/phonenumbers/phonenumbe...
> File cpp/test/phonenumbers/phonenumbermatcher_test.cc (right):
> 
>
http://codereview.appspot.com/5627046/diff/1/cpp/test/phonenumbers/phonenumbe...
> cpp/test/phonenumbers/phonenumbermatcher_test.cc:531:
> EXPECT_TRUE(IsLatinLetter(UnicodeString("\xCC\x81")[0]));
> On 2012/02/04 23:56:44, leandrogracia wrote:
> > fromUTF8 is required here too.
> 
> Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5