Review Request: KAuth support in KDM kcontrol module

[Oswald Buddenhagen]

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


it's starting to look really good - i have only nitpicks left. :-)


/trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-dlg.cpp
<http://reviewboard.kde.org/r/3631/#comment4642>

    i won't insist on doing that, but you could copy remote files to a temporary location.



/trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-users.cpp
<http://reviewboard.kde.org/r/3631/#comment4644>

    the code for the helper calls is very repetetive. i would factor it out to a global function in main.cpp.
    the global method would take the action name, the variantmap and a qwidget parent pointer and return the reply error code if it was not handled yet.



/trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-users.cpp
<http://reviewboard.kde.org/r/3631/#comment4647>

    this disables only the current tab, so one could still switch the tab, right? if so, then you should probably use parentWidget().



/trunk/KDE/kdebase/workspace/kcontrol/kdm/main.cpp
<http://reviewboard.kde.org/r/3631/#comment4643>

    this isn't really exclusive.
    though you wouldn't want to display two dialogs in direct succession, either.
    maybe create a single joined message.


- Oswald


On 2010-04-21 16:45:15, Igor Krivenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3631/
> -----------------------------------------------------------
> 
> (Updated 2010-04-21 16:45:15)
> 
> 
> Review request for kdelibs, usability, Dario Freddi, Oswald Buddenhagen, and Frederik Gladhorn.
> 
> 
> Summary
> -------
> 
> This is a patch for KDM kcontrol module.
> It implements Kauth support for changing kdm settings and managing user images. 
> It still doesn't support installation/deletion of themes (including installation via knewstuff).
> As far as I know there were plans to port KNewStuff3 to KAuth, so a more consistent approach is
> to rely on the KNewStuff's integration when it's available.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/CMakeLists.txt 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/background.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/background.cpp 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/helper.h PRE-CREATION 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/helper.cpp PRE-CREATION 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kcmkdm_actions.actions PRE-CREATION 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-conv.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-conv.cpp 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-dlg.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-dlg.cpp 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-gen.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-gen.cpp 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-shut.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-shut.cpp 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-theme.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-theme.cpp 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-users.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-users.cpp 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/main.h 1113083 
>   /trunk/KDE/kdebase/workspace/kcontrol/kdm/main.cpp 1113083 
> 
> Diff: http://reviewboard.kde.org/r/3631/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Saving KDM settings
>   http://reviewboard.kde.org/r/3631/s/360/
> Managing user images
>   http://reviewboard.kde.org/r/3631/s/361/
> 
> 
> Thanks,
> 
> Igor
> 
>