Review Request: check in AddAccountAssistant for installed connection managers and optional install them

[David Edmundson]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20121229/11205777/attachment.html>