Review Request: Mobile Connection Wizard: A concept of i18n of APN's list

[Lamarque Vieira Souza]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104349/#review11630
-----------------------------------------------------------



settings/config/mobileconnectionwizard.h
<http://git.reviewboard.kde.org/r/104349/#comment9207>

    remove trailing whitespace.
    
    This patch is against nm09 branch. Can you provide a patch against master branch too?
    
    I do not have time to test the patch now. I will do it next weekend.



settings/config/mobileconnectionwizard.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9222>

    You removed insertSeparator in the "else" clause, why not here? This can leads to a crash in the wizard.



settings/config/mobileconnectionwizard.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9209>

    Use "foreach (const QVariantMap &apn, mApns)", it's more efficient.



settings/config/mobileconnectionwizard.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9216>

    Use ')' (single quotes) instead of ")" (double quotes), it's more efficient when handling a single character.



settings/config/mobileconnectionwizard.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9217>

    stick to the code style and use four spaces for indentation.



settings/config/mobileconnectionwizard.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9218>

    stick to the code style and use "} else {" here.



settings/config/mobileproviders.h
<http://git.reviewboard.kde.org/r/104349/#comment9219>

    Forcing everyone to pass a QDomElement to this method is not a good ideia API speaking. This forces everybody to first parse the file before they can get the QDomElement.



settings/config/mobileproviders.h
<http://git.reviewboard.kde.org/r/104349/#comment9211>

    remote trailing whitespace.



settings/config/mobileproviders.h
<http://git.reviewboard.kde.org/r/104349/#comment9215>

    remove trailing whitespace.



settings/config/mobileproviders.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9214>

    remove trailing whitespace.



settings/config/mobileproviders.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9212>

    remote trailing whitespace.



settings/config/mobileproviders.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9213>

    remote trailing writespace.



settings/config/mobileproviders.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9221>

    Use single quotes for single characters, it's more efficient.



settings/config/mobileproviders.cpp
<http://git.reviewboard.kde.org/r/104349/#comment9220>

    What is the problem here? Stick to the code style, add a space after the comma.


- Lamarque Vieira Souza


On March 19, 2012, 6:55 p.m., Swami Dhyan Nataraj [Nikolay Shaplov] wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104349/
> -----------------------------------------------------------
> 
> (Updated March 19, 2012, 6:55 p.m.)
> 
> 
> Review request for Network Management and Lamarque Vieira Souza.
> 
> 
> Description
> -------
> 
> This is a kind of concept how to internationalize list of APNs in mobile connection wizard.
> 
> This concept uses less CPU and more memory (through this task is not resourse critical). It create a list of apn's QVatiant map once, and then uses it all the time till the end. I thought it is better then reparse XML each time we need an APL info.
> 
> If there are ant comments, I will try to do fixes. If no, and everything is OK, I will remove all commented out code from the patch, and let's commit it :-)
> 
> 
> Diffs
> -----
> 
>   settings/config/mobileconnectionwizard.h 3cd801e92f1bf15acf7857192fac052107fdfe52 
>   settings/config/mobileconnectionwizard.cpp b2e72d631b8272c4be5bdb8766a1d214715e2038 
>   settings/config/mobileproviders.h 8df2269b38f7339d328de53e0be4734896bc1595 
>   settings/config/mobileproviders.cpp 24d2c9256ebc87997e8a9f1903faa84900aa4d16 
> 
> Diff: http://git.reviewboard.kde.org/r/104349/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Swami Dhyan Nataraj [Nikolay Shaplov]
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120319/d458d9a2/attachment-0001.html>