Port kcm cursor to KConfigXT
ClosedPublic

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

Details

Summary
  • Allow code reuse

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 17483
Build 17501: arc lint + arc unit
bport created this revision.Oct 9 2019, 2:36 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 9 2019, 2:36 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
bport requested review of this revision.Oct 9 2019, 2:36 PM
davidedmundson added inline comments.
kcms/cursortheme/CMakeLists.txt
22

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.Oct 9 2019, 2:54 PM
This revision is now accepted and ready to land.Oct 9 2019, 2:54 PM
bport added inline comments.Oct 9 2019, 3:00 PM
kcms/cursortheme/CMakeLists.txt
22

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.Oct 9 2019, 8:09 PM
ervin added inline comments.
kcms/cursortheme/CMakeLists.txt
22

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

kcms/cursortheme/cursorthemesettings.kcfg
9

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.Oct 9 2019, 8:09 PM
bport updated this revision to Diff 67605.Oct 10 2019, 1:48 PM
  • Install kcfg file
  • Remove uneeded include from kcfg file
bport marked 3 inline comments as done.Oct 10 2019, 1:52 PM
ervin requested changes to this revision.Oct 15 2019, 8:49 AM

Just a last nitpick.

kcms/cursortheme/CMakeLists.txt
24

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.Oct 15 2019, 8:49 AM
bport updated this revision to Diff 67948.Oct 15 2019, 9:25 AM

Move install line to the good section

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