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

[Swami Dhyan Nataraj [Nikolay Shaplov]]

> On March 19, 2012, 8:01 p.m., Lamarque Vieira Souza wrote:
> > settings/config/mobileproviders.h, line 51
> > <http://git.reviewboard.kde.org/r/104349/diff/1/?file=53850#file53850line51>
> >
> >     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.

I considered it as an internal function that allows to convert xml representation into QVariantMap one. It should not be used outside of the object. 
getApns already gives a list of QVariantMaps.

May be it would be good to move getApnInfo into private area and change name to apnXml2VariantMan or something like this.


> On March 19, 2012, 8:01 p.m., Lamarque Vieira Souza wrote:
> > settings/config/mobileconnectionwizard.cpp, line 150
> > <http://git.reviewboard.kde.org/r/104349/diff/1/?file=53849#file53849line150>
> >
> >     You removed insertSeparator in the "else" clause, why not here? This can leads to a crash in the wizard.

Oups, my mistake


> On March 19, 2012, 8:01 p.m., Lamarque Vieira Souza wrote:
> > settings/config/mobileconnectionwizard.cpp, line 162
> > <http://git.reviewboard.kde.org/r/104349/diff/1/?file=53849#file53849line162>
> >
> >     Use "foreach (const QVariantMap &apn, mApns)", it's more efficient.

Fixed. But I can't understand the idea. What this will give to us? 
const will forbid changing of the apn but how it will help us?


> On March 19, 2012, 8:01 p.m., Lamarque Vieira Souza wrote:
> > settings/config/mobileproviders.cpp, line 276
> > <http://git.reviewboard.kde.org/r/104349/diff/1/?file=53851#file53851line276>
> >
> >     Use single quotes for single characters, it's more efficient.

It was not me' I've just copied code from function above :-)


- Swami Dhyan Nataraj


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


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/20120331/d34f502f/attachment.html>