----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107995/#review24156 -----------------------------------------------------------
Looks good \o/ Missing license is the only thing that's really wrong. I'd like to leave the review here for a day or two to discuss the new strings, then you'll get a ship it from me. src/KCMTelepathyAccounts/package-install-action.h <http://git.reviewboard.kde.org/r/107995/#comment18479> License src/KCMTelepathyAccounts/package-install-action.cpp <http://git.reviewboard.kde.org/r/107995/#comment18480> License src/KCMTelepathyAccounts/package-install-action.cpp <http://git.reviewboard.kde.org/r/107995/#comment18488> #include <KLocalizedString> src/KCMTelepathyAccounts/package-install-action.cpp <http://git.reviewboard.kde.org/r/107995/#comment18489> As a general rule, synchronous dbus calls are banned as they suck. However as this will be called so rarely, I'll let you get away with it. Don't make a habit of it though :) src/KCMTelepathyAccounts/package-install-action.cpp <http://git.reviewboard.kde.org/r/107995/#comment18490> Nitpick, technically coding style is if () { } else { } src/KCMTelepathyAccounts/simple-profile-select-widget.cpp <http://git.reviewboard.kde.org/r/107995/#comment18487> I think maybe we should go with: "Your setup misses some optional telepathy plugin" (with the word optional added) Or maybe we should explain what these plugins do "In order to enable all possible accounts, further plugins must be installed". Discuss. - David Edmundson On Dec. 29, 2012, 12:04 a.m., Florian Reinhard wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107995/ > ----------------------------------------------------------- > > (Updated Dec. 29, 2012, 12:04 a.m.) > > > Review request for Telepathy. > > > Description > ------- > > Check in SimpleProfileSelectWidget if required connection managers are > installed > > * new cmake option -DPACKAGE_INSTALLATION=[apt,packagekit] > * default: no package installation enabled just promt a message > * on the first page of AddAccountAssistant profiles with no CM installed will > be disabled > > > Diffs > ----- > > src/KCMTelepathyAccounts/CMakeLists.txt > e19f3accf465f852012d251f69bbfa2ff372bb7f > src/KCMTelepathyAccounts/package-install-action.h PRE-CREATION > src/KCMTelepathyAccounts/package-install-action.cpp PRE-CREATION > src/KCMTelepathyAccounts/profile-list-model.h > 8b313d19765b63071408047b5d183ecc419500de > src/KCMTelepathyAccounts/profile-list-model.cpp > 12752928f377552a323a502557d2717819c257de > src/KCMTelepathyAccounts/simple-profile-select-widget.h > 52eede1c4d5b7a39143b71c19f33dcb965827bf9 > src/KCMTelepathyAccounts/simple-profile-select-widget.cpp > 5f9a516e74a17c8cf86a5f5382d5071baff1c8a8 > src/KCMTelepathyAccounts/simple-profile-select-widget.ui > 5ad8e79a7bd4fd6d52dac61cbd88becb0cd569ae > > Diff: http://git.reviewboard.kde.org/r/107995/diff/ > > > Testing > ------- > > * remove telepathy-haze > * start the kcm > * result see screenshot > > > Screenshots > ----------- > > telepathy-haze not installed > http://git.reviewboard.kde.org/r/107995/s/937/ > > > Thanks, > > Florian Reinhard > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
