Review Request: KAuth support in KDM kcontrol module
Oswald Buddenhagen
ossi at kde.org
Sat Apr 17 15:20:44 BST 2010
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3631/#review5085
-----------------------------------------------------------
hi igor. this looks like a good start. :)
Screenshot: Saving KDM settings
<http://reviewboard.kde.org//r/3631/#scomment40>
obviously, the contents of the vendor field belong under application. this seems to be a framework bug, though.
/trunk/KDE/kdebase/workspace/kcontrol/kdm/helper.cpp
<http://reviewboard.kde.org/r/3631/#comment4564>
you cannot pass the system config locations through arguments, as that opens the door to using the action+helper for overwriting arbitrary files.
in fact, the tutorial on techbase literally prods one into committing this mistake ... :(
/trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-conv.cpp
<http://reviewboard.kde.org/r/3631/#comment4565>
the code is sprinkled with read-only mode checks like this all over the place. you should seek out and remove them.
/trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-dlg.cpp
<http://reviewboard.kde.org/r/3631/#comment4566>
new line here please
/trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-users.cpp
<http://reviewboard.kde.org/r/3631/#comment4567>
this is an unrelated change. while i'll happily accept it (though it needs to be done at least in genkdmconf.c as well, and i'd like to hear a justification), it needs to go to a separate patch.
/trunk/KDE/kdebase/workspace/kcontrol/kdm/kdm-users.cpp
<http://reviewboard.kde.org/r/3631/#comment4568>
no need to assign an empty string. this just bloats the code.
/trunk/KDE/kdebase/workspace/kcontrol/kdm/main.h
<http://reviewboard.kde.org/r/3631/#comment4569>
you can forward-declare kconfig instead
/trunk/KDE/kdebase/workspace/kcontrol/kdm/main.cpp
<http://reviewboard.kde.org/r/3631/#comment4570>
dead debug code
- Oswald
On 2010-04-17 12:22:47, Igor Krivenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3631/
> -----------------------------------------------------------
>
> (Updated 2010-04-17 12:22:47)
>
>
> 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
>
>
More information about the kde-core-devel
mailing list