----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101275/#review3111 -----------------------------------------------------------
libs/internals/connection.h <http://git.reviewboard.kde.org/r/101275/#comment2636> Please rename this method to saveCertificates and the one below to deleteCertificates to make clear what they are saving and deleting. libs/internals/settings/802-1x.h <http://git.reviewboard.kde.org/r/101275/#comment2644> addToCertToDelete libs/internals/settings/802-1x.h <http://git.reviewboard.kde.org/r/101275/#comment2643> removeFromCertToDelete libs/internals/settings/802-1x.h <http://git.reviewboard.kde.org/r/101275/#comment2638> I thinks certPathAsByteArray sounds better, just my oppinion. libs/internals/settings/802-1x.h <http://git.reviewboard.kde.org/r/101275/#comment2639> use QString() instead of "". libs/internals/settings/802-1x.h <http://git.reviewboard.kde.org/r/101275/#comment2640> Why the parameter scope is int instead of enumerate? libs/internals/settings/802-1x.cpp <http://git.reviewboard.kde.org/r/101275/#comment2646> stick to the code sytle for if { \n } else { \n } libs/internals/settings/802-1x.cpp <http://git.reviewboard.kde.org/r/101275/#comment2647> code style: move the { below to this line. libs/internals/settings/802-1x.cpp <http://git.reviewboard.kde.org/r/101275/#comment2648> code style for if () { in this entire method. libs/ui/security/peapwidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2649> code style for if () { \n } else { \n } libs/ui/security/peapwidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2651> code style for if. libs/ui/security/peapwidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2650> code style for if. libs/ui/security/tlswidget.h <http://git.reviewboard.kde.org/r/101275/#comment2652> wrong indentation in this line and below. libs/ui/security/tlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2645> Stick to the code style: if { \n } else { \n } libs/ui/security/tlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2653> code style for if libs/ui/security/tlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2656> code style for if { \n } else { \n } libs/ui/security/tlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2641> I use QLatin1String for string constants. libs/ui/security/tlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2654> code style for if. libs/ui/security/ttlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2642> Stick to the code style and use: if () { \n } else { \n } libs/ui/security/ttlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2655> code style for if libs/ui/security/ttlswidget.cpp <http://git.reviewboard.kde.org/r/101275/#comment2657> code stytle for if - Lamarque Vieira On May 2, 2011, 2:30 p.m., Ilia Kats wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101275/ > ----------------------------------------------------------- > > (Updated May 2, 2011, 2:30 p.m.) > > > Review request for Network Management. > > > Summary > ------- > > Currently, KDE NM is not compatible with PEM certificates (base64-encoded), > which are the most used format. Attached patch makes some improvements in > certificate handling, namely: > until now, the certificate was stored in the connection settings file, not > base64-decoded or anything, and passed like that to NM. However, NM expects a > binary DER certificate if using the blob scheme or a path to the certificate > in the form of file://path_to_certificate if using the path scheme. Since a > PEM file containing multiple certificates is possible, we would have to use > the path scheme anyway, as a DER blob can only contain one certificate, so > this patch also improves the certificate handling: Instead of just saving the > path to the certificate, it is now being imported to > $HOME/.kde/share/networkmanagement/certificates for user scope connections or > /usr/share/kde4/apps/networkmanagement/certificates for systemscope > connections. This allows the user to move or delete the original file without > worrying about his connections. Certificates are deleted when they are no > longer needed, e.g. the connection gets deleted, "Use system CA" is checked > or another security method is selected. > > This is a major change in behavior, so I would like a public review. > > > This addresses bug 209673. > http://bugs.kde.org/show_bug.cgi?id=209673 > > > Diffs > ----- > > libs/internals/connectionpersistence.cpp 1a22c5b > libs/internals/setting.h b43365b > libs/internals/setting.cpp a637855 > libs/internals/settings/802-1x.h 971ef41 > libs/internals/connection.cpp 701a9e2 > CMakeLists.txt 6748cee > backends/NetworkManager/nmdbussettingsconnectionprovider.cpp 40a364f > libs/internals/connection.h 60516dc > libs/internals/settings/802-1x.cpp 245507e > libs/internals/settings/802-1xpersistence.cpp b7b9c42 > libs/ui/security/eapmethodleap.cpp 199a8b8 > libs/ui/security/eapmethodpeapbase.ui 4b9c5fd > libs/ui/security/eapmethodtlsbase.ui 80a5c1f > libs/ui/security/eapmethodttlsbase.ui 4f2e1a9 > libs/ui/security/peapwidget.h 51ad781 > libs/ui/security/peapwidget.cpp 2fc502b > libs/ui/security/tlswidget.h c71dcf3 > libs/ui/security/tlswidget.cpp 1d28636 > libs/ui/security/ttlswidget.h b1a9049 > libs/ui/security/ttlswidget.cpp c135b19 > settings/config/manageconnectionwidget.h 58afc12 > settings/config/manageconnectionwidget.cpp 3c0d4dc > > Diff: http://git.reviewboard.kde.org/r/101275/diff > > > Testing > ------- > > Yes, see the bugzilla ticket. > > > Thanks, > > Ilia > >
_______________________________________________ kde-networkmanager mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-networkmanager
