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

Issue 14930044: CPP: Remove uses of std::{cerr,endl} in production code. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months, 1 week ago by pliard1
Modified:
6 months, 1 week ago
Reviewers:
thakis, roes
CC:
shaopengjia, yosin
Base URL:
https://libphonenumber.googlecode.com/svn/trunk
Visibility:
Public.

Description

CPP: Remove uses of std::{cerr,endl} in production code.

They generated static initializers on Clang/Mac.

This also adds the previously missing <algorithm> include in unicodetext.cc
causing a compilation error on VS2013.

This is a squash of (very slightly modified) patches contributed by thakis@ and
yosin@.

R=roes@google.com

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

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -25 lines) Patch
M cpp/src/phonenumbers/phonenumbermatcher.cc View 3 chunks +1 line, -4 lines 0 comments Download
M cpp/src/phonenumbers/phonenumberutil.cc View 3 chunks +2 lines, -9 lines 0 comments Download
M cpp/src/phonenumbers/shortnumberinfo.cc View 1 chunk +1 line, -3 lines 0 comments Download
M cpp/src/phonenumbers/utf/unicodetext.cc View 6 chunks +7 lines, -9 lines 2 comments Download

Messages

Total messages: 9
pliard1
6 months, 1 week ago #1
roes
On 2013/10/18 08:55:18, pliard1 wrote: LGTM. Not that familiar with the c++ library, but looks ...
6 months, 1 week ago #2
roes
https://codereview.appspot.com/14930044/diff/1/cpp/src/phonenumbers/utf/unicodetext.cc File cpp/src/phonenumbers/utf/unicodetext.cc (right): https://codereview.appspot.com/14930044/diff/1/cpp/src/phonenumbers/utf/unicodetext.cc#newcode234 cpp/src/phonenumbers/utf/unicodetext.cc:234: fprintf(stderr, "UTF-8 buffer is not interchange-valid.\n"); Is there a ...
6 months, 1 week ago #3
pliard1
Thanks Cecilia! https://codereview.appspot.com/14930044/diff/1/cpp/src/phonenumbers/utf/unicodetext.cc File cpp/src/phonenumbers/utf/unicodetext.cc (right): https://codereview.appspot.com/14930044/diff/1/cpp/src/phonenumbers/utf/unicodetext.cc#newcode234 cpp/src/phonenumbers/utf/unicodetext.cc:234: fprintf(stderr, "UTF-8 buffer is not interchange-valid.\n"); On ...
6 months, 1 week ago #4
roes
Thanks for the explanation. On Fri, Oct 18, 2013 at 11:53 AM, <pliard@chromium.org> wrote: > ...
6 months, 1 week ago #5
pliard1
Committed patchset #1 manually as r621 (presubmit successful).
6 months, 1 week ago #6
thakis
For what it's worth, this isn't specific to Mac nor Clang. Including <iostream> inserts a ...
6 months, 1 week ago #7
thakis
…and thanks for landing! On Fri, Oct 18, 2013 at 2:56 AM, <pliard@chromium.org> wrote: > ...
6 months, 1 week ago #8
pliard1
6 months, 1 week ago #9
Message was sent while issue was closed.
On 2013/10/18 14:26:17, thakis wrote:
> …and thanks for landing!
> 
> 
> On Fri, Oct 18, 2013 at 2:56 AM, <mailto:pliard@chromium.org> wrote:
> 
> > Committed patchset #1 manually as r621 (presubmit successful).
> >
> >
>
https://codereview.appspot.**com/14930044/%3Chttps://codereview.appspot.com/1...>
> >

Ah thanks for the info! I will prepare a roll on the Chromium side.
Sign in to reply to this message.

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