- Clean a little the code
- Improve a little the UI
Details
Diff Detail
- Repository
- R108 KWin
- Branch
- arcpatch-D27955
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 23972 Build 23990: arc lint + arc unit
kcmkwin/kwincompositing/compositing.cpp | ||
---|---|---|
55 | Naming nitpick: onChanged isn't really a great name for a signal. Usually, "on" is used with slots. What about connecting Compositing::animationSpeedChanged to Compositing::updateSettings? e.g. connect(this, &Compositing::animationSpeedChanged, this, &Compositing::updateSettings); | |
287–296 | Given that m_changed is not set to true when anything has been changed, there is a chance that we won't send a reinit request. We probably need to save the return value of m_settings->isSaveNeeded() before calling m_settings->save() and check it here. |
kcmkwin/kwincompositing/compositing.h | ||
---|---|---|
93 | We probably don't need this signal anymore. |
kcmkwin/kwincompositing/compositing.cpp | ||
---|---|---|
80 | Heh, - 4 and + 4 were obscure before so this patch doesn't make things better or worse. Just for context, we add and subtract 4 because long time ago HiddenPreviews had values with different meaning than what we have now. In order to break backwards compatibility, the new values were shifted by 4. |
Use key attribute from item prefixed by gl, add a comment about magic -4 HiddenPreviews
Overall, +1.
kcmkwin/kwincompositing/compositing.cpp | ||
---|---|---|
53 | Shouldn't the parent be this? | |
kcmkwin/kwincompositing/compositing.h | ||
104–107 | naming nit: could you please rename these methods to something else? It's unclear what values applyValues applies. What about these? void updateSettingsFromUi(); void updateUiFromSettings(); Feel free to ignore this comment because it looks like these methods will go away in D27988 |
kcmkwin/kwincompositing/compositing.cpp | ||
---|---|---|
80 | And we didn't wrap that in conversion functions? :-) |