Details
- Reviewers
ervin mart bport - Group Reviewers
Plasma - Commits
- R119:77b27da43cd2: Port KCMSplashScreen to KConfigXT
Diff Detail
- Repository
- R119 Plasma Desktop
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 18034 Build 18052: arc lint + arc unit
kcms/ksplash/CMakeLists.txt | ||
---|---|---|
29 | You don't need to install the kcfg file if you're generating it. |
kcms/ksplash/CMakeLists.txt | ||
---|---|---|
29 | kcfg file is not generated, it is used to generate class SplashScreenSettings It was requested by davidedmundson on another review, thought I should add it as well. |
Bunch of comments, not formally requesting changes since for a couple of them I'm unsure if I missed something specific to the module being at play.
kcms/ksplash/CMakeLists.txt | ||
---|---|---|
29 | I'm still not quite sure why it was requested to be honest. The reusability potential of those kcfgs is rather low in most cases. | |
kcms/ksplash/kcm.cpp | ||
138 | I'd expect this to be unnecessary since load() will emit ThemeChanged or EngineChanged if said config keys really changed. | |
145 | I'd expect either setNeedsSave(false) here, or configChanged() to be connected from the ctor. Are we sure that state is reset properly on apply being clicked? | |
kcms/ksplash/splashscreensettings.kcfg | ||
12 | What about using the name elements for both entries to avoid ending up with signals starting with an uppercase? (although one could argue that kconfig_compiler should be fixed instead... it somehow lowercases properly the property and getter but not the signal). |