CPP: libphonenumber 4.2.

CPP: libphonenumber 4.2.

Committed: http://code.google.com/p/libphonenumber/source/detail?r=387

[lararennie, 2011-11-10 11:36:11]

http://codereview.appspot.com/5372051/diff/1/cpp/src/phonenumbers/phonenumber...
File cpp/src/phonenumbers/phonenumbermatcher.h (right):

http://codereview.appspot.com/5372051/diff/1/cpp/src/phonenumbers/phonenumber...
cpp/src/phonenumbers/phonenumbermatcher.h:58: // level to use, talk to
intl-eng-emea@google.com.
Um, we don't want to have this comment :) Say something about mailing the
mailing list (external)

[philippe, 2011-11-10 11:41:39]

http://codereview.appspot.com/5372051/diff/1/cpp/src/phonenumbers/phonenumber...
File cpp/src/phonenumbers/phonenumbermatcher.h (right):

http://codereview.appspot.com/5372051/diff/1/cpp/src/phonenumbers/phonenumber...
cpp/src/phonenumbers/phonenumbermatcher.h:58: // level to use, talk to
intl-eng-emea@google.com.
On 2011/11/10 11:36:11, lararennie wrote:
> Um, we don't want to have this comment :) Say something about mailing the
> mailing list (external)

Indeed :)

[lararennie, 2011-11-11 13:30:52]

http://codereview.appspot.com/5372051/diff/1013/cpp/src/phonenumbers/phonenum...
File cpp/src/phonenumbers/phonenumberutil.cc (right):

http://codereview.appspot.com/5372051/diff/1013/cpp/src/phonenumbers/phonenum...
cpp/src/phonenumbers/phonenumberutil.cc:1812: GetRegionCodeForNumber(number,
&region_code);
This changed in Java too. Did we not change it in internal C++? We probably
should, for consistency and speed...

[philippe, 2011-11-11 15:00:51]

http://codereview.appspot.com/5372051/diff/1013/cpp/src/phonenumbers/phonenum...
File cpp/src/phonenumbers/phonenumberutil.cc (right):

http://codereview.appspot.com/5372051/diff/1013/cpp/src/phonenumbers/phonenum...
cpp/src/phonenumbers/phonenumberutil.cc:1812: GetRegionCodeForNumber(number,
&region_code);
On 2011/11/11 13:30:52, lararennie wrote:
> This changed in Java too. Did we not change it in internal C++? We probably
> should, for consistency and speed...

Yes we have that internally. I missed this part probably due to the RegionCode
difference. I also missed a part in FormatMobileForDialling.
I am looking forward to having RegionCode used at least in this cc file :)

[lararennie, 2011-11-14 12:19:40]

I think these are the last things.

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
File cpp/src/phonenumbers/phonenumbermatcher.cc (right):

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
cpp/src/phonenumbers/phonenumbermatcher.cc:371: // return the extra values
parsed. TODO(lararennie): stop clearing all
Remove username from TODO

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
File cpp/src/phonenumbers/phonenumbermatcher.h (right):

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
cpp/src/phonenumbers/phonenumbermatcher.h:58: // level to use, you can send an
e-mail to the discussion group.
Actually include the email address. We do this in some other comment somewhere -
perhaps the main PhoneNumberUtil class? maybe the .java version?

http://codereview.appspot.com/5372051/diff/7017/debian/changelog
File debian/changelog (right):

http://codereview.appspot.com/5372051/diff/7017/debian/changelog#newcode5
debian/changelog:5: -- Fredrik Roubert <roubert@google.com>  Thu, 10 Nov 2011
12:30:28 +0200
May need to update date here for when you actually submit...?

[philippe, 2011-11-14 13:12:33]

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
File cpp/src/phonenumbers/phonenumbermatcher.cc (right):

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
cpp/src/phonenumbers/phonenumbermatcher.cc:371: // return the extra values
parsed. TODO(lararennie): stop clearing all
On 2011/11/14 12:19:40, lararennie wrote:
> Remove username from TODO

Done.

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
File cpp/src/phonenumbers/phonenumbermatcher.h (right):

http://codereview.appspot.com/5372051/diff/7017/cpp/src/phonenumbers/phonenum...
cpp/src/phonenumbers/phonenumbermatcher.h:58: // level to use, you can send an
e-mail to the discussion group.
On 2011/11/14 12:19:40, lararennie wrote:
> Actually include the email address. We do this in some other comment somewhere
-
> perhaps the main PhoneNumberUtil class? maybe the .java version?

Done.

http://codereview.appspot.com/5372051/diff/7017/debian/changelog
File debian/changelog (right):

http://codereview.appspot.com/5372051/diff/7017/debian/changelog#newcode5
debian/changelog:5: -- Fredrik Roubert <roubert@google.com>  Thu, 10 Nov 2011
12:30:28 +0200
On 2011/11/14 12:19:40, lararennie wrote:
> May need to update date here for when you actually submit...?

Done.

[lararennie, 2011-11-14 13:53:39]

LGTM