[Merge] lp:~tiagosh/telepathy-ofono/use-libphonenumber into lp:telepathy-ofono

[Gustavo Pichorim Boiko]

Review: Needs Fixing

Just a couple remarks, other than that looks good.

Diff comments:

> 
> === modified file 'phoneutils.cpp'
> --- phoneutils.cpp	2015-06-17 20:19:50 +0000
> +++ phoneutils.cpp	2015-08-05 15:54:59 +0000
> @@ -1,8 +1,9 @@
>  /*
> - * Copyright (C) 2012-2013 Canonical, Ltd.
> + * Copyright (C) 2015 Canonical, Ltd.

As this is not a new file, it is just that it changed a lot, I think you should keep the 2012-2015 copyright years

>   *
>   * Authors:
>   *  Gustavo Pichorim Boiko <gustavo.boiko at canonical.com>
> + *  Renato Araujo Oliveira Filho <renato.filho at canonical.com>
>   *  Tiago Salem Herrmann <tiago.herrmann at canonical.com>
>   *
>   * This file is part of telepathy-ofono.
> 
> === modified file 'tests/PhoneUtilsTest.cpp'
> --- tests/PhoneUtilsTest.cpp	2013-12-04 14:31:39 +0000
> +++ tests/PhoneUtilsTest.cpp	2015-08-05 15:54:59 +0000
> @@ -67,18 +68,15 @@
>      QTest::newRow("string equal") << "12345678" << "12345678" << true;
>      QTest::newRow("number with dash") << "1234-5678" << "12345678" << true;
>      QTest::newRow("number with area code") << "12312345678" << "12345678" << true;
> -    QTest::newRow("number with extension") << "12345678#123" << "12345678" << false;
> +    QTest::newRow("number with extension") << "12345678#123" << "12345678" << true;
>      QTest::newRow("both numbers with extension") << "(123)12345678#1" << "12345678#1" << true;
>      QTest::newRow("numbers with different extension") << "1234567#1" << "1234567#2" << false;
> -    QTest::newRow("number with comma") << "33333333,1,1" << "33333333" << true;
> -    QTest::newRow("both numbers with comma") << "22222222,1" << "22222222,2,1" << true;
> -    QTest::newRow("number with semicolon") << "33333333;1" << "33333333" << true;
> -    QTest::newRow("both numbers with semicolon") << "22222222;1" << "22222222;2" << true;
>      QTest::newRow("short/emergency numbers") << "190" << "190" << true;
>      QTest::newRow("different numbers") << "12345678" << "1234567" << false;
>      QTest::newRow("both non phone numbers") << "abcdefg" << "abcdefg" << true;
>      QTest::newRow("different non phone numbers") << "abcdefg" << "bcdefg" << false;
> -    QTest::newRow("phone number and custom string") << "abc12345678" << "12345678" << false;
> +    QTest::newRow("phone number and custom string") << "abc12345678" << "12345678" << true;
> +    QTest::newRow("phone number with dots") << "+31 (475) 12.34.56" << "+31 (475) 12 34 56" << true;

Can you add a test case of numbers with slashes just to cover bug #1462090 ?

>      // FIXME: check what other cases we need to test here"
>  }
>  


-- 
https://code.launchpad.net/~tiagosh/telepathy-ofono/use-libphonenumber/+merge/264905
Your team Ubuntu Phablet Team is subscribed to branch lp:telepathy-ofono.