Port kcm cursor to KConfigXT
ClosedPublic

Authored by bport on Wed, Oct 9, 2:36 PM.

Details

Summary
  • Allow code reuse

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bport created this revision.Wed, Oct 9, 2:36 PM
Restricted Application added a project: Plasma. · View Herald TranscriptWed, Oct 9, 2:36 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
bport requested review of this revision.Wed, Oct 9, 2:36 PM
davidedmundson added inline comments.
kcms/cursortheme/CMakeLists.txt
23–24

Can you also add

install(FILES kded/touchpaddaemon.kcfg
        DESTINATION ${KDE_INSTALL_KCFGDIR})

This doesn't explicitly do anything useful, but it allows future metadata extraction for documentation purposes.

kcms/cursortheme/kcmcursortheme.cpp
72

I assume Kevin's patch set fixes this?

davidedmundson accepted this revision.Wed, Oct 9, 2:54 PM
This revision is now accepted and ready to land.Wed, Oct 9, 2:54 PM
bport added inline comments.Wed, Oct 9, 3:00 PM
kcms/cursortheme/CMakeLists.txt
23–24

Ok I will add it

kcms/cursortheme/kcmcursortheme.cpp
72

Yes but kevin patch is not yet merged.
However we will need to go throught this code a second time after Kevin's patch.

ervin requested changes to this revision.Wed, Oct 9, 8:09 PM
ervin added inline comments.
kcms/cursortheme/CMakeLists.txt
23–24

Interestingly you didn't request that on my desktoptheme work.

kcms/cursortheme/cursorthemesettings.kcfg
10

Why those includes? They seem unused now since you're not having code in defaults.

kcms/cursortheme/kcmcursortheme.cpp
72

Yes, giving a bit more time to people to review the latest revisions of my KConfig patches. They changed quite a bit and I didn't get further comment yet.

kcms/cursortheme/kcmcursortheme.h
127

Why the empty line removal? The previous line is now stuck with the comment.

This revision now requires changes to proceed.Wed, Oct 9, 8:09 PM
bport updated this revision to Diff 67605.Thu, Oct 10, 1:48 PM
  • Install kcfg file
  • Remove uneeded include from kcfg file
bport marked 3 inline comments as done.Thu, Oct 10, 1:52 PM
ervin requested changes to this revision.Tue, Oct 15, 8:49 AM

Just a last nitpick.

kcms/cursortheme/CMakeLists.txt
25

Could that be moved in the install file section of the file? Everything related to file installs (but not target installs...) seems to be at the end of that file.

This revision now requires changes to proceed.Tue, Oct 15, 8:49 AM
bport updated this revision to Diff 67948.Tue, Oct 15, 9:25 AM

Move install line to the good section

ervin accepted this revision.Tue, Oct 15, 9:27 AM
This revision is now accepted and ready to land.Tue, Oct 15, 9:27 AM
This revision was automatically updated to reflect the committed changes.