----------------------------------------------------------- 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] > >
_______________________________________________ kde-networkmanager mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-networkmanager
