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 17979 Build 17997: arc lint + arc unit
| kcms/ksplash/CMakeLists.txt | ||
|---|---|---|
| 29 ↗ | (On Diff #68130) | You don't need to install the kcfg file if you're generating it. |
| kcms/ksplash/CMakeLists.txt | ||
|---|---|---|
| 29 ↗ | (On Diff #68130) | 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 ↗ | (On Diff #68130) | 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 | ||
| 140 ↗ | (On Diff #68130) | I'd expect this to be unnecessary since load() will emit ThemeChanged or EngineChanged if said config keys really changed. |
| 147 ↗ | (On Diff #68130) | 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 | ||
| 11 ↗ | (On Diff #68130) | 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). |