Review Request 129872: [lookandfeel] Support changing window decoration

Martin Gräßlin mgraesslin at kde.org
Sat Jan 28 08:47:13 UTC 2017


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129872/#review102292
-----------------------------------------------------------



I would have a much better time understanding your change if you would update the description and an explanation. Currently it says "Support changing window decoration" which your change does not do. And you have no explanation at all. I'm trying to understand your change, but I cannot. I do not understand any of the changes you do. Please explain, so that others can understand it and determine whether it's correct or not. Currently my assumption is, that it's not correct, because there are changes looking wrong to me, like introduction of methods which are not called, removal of code to reconfigure KWin, etc. etc.


kcms/lookandfeel/kcm.cpp (line 246)
<https://git.reviewboard.kde.org/r/129872/#comment68282>

    what about theme?



kcms/lookandfeel/kcm.cpp (line 362)
<https://git.reviewboard.kde.org/r/129872/#comment68283>

    I don't understand why the group is changed here? Was if wrong before? How was it wrong?



kcms/lookandfeel/kcm.cpp 
<https://git.reviewboard.kde.org/r/129872/#comment68284>

    If this is no longer needed, why put it together with an unrelated change?



kcms/lookandfeel/kcm.cpp (lines 759 - 766)
<https://git.reviewboard.kde.org/r/129872/#comment68285>

    what is this method for? Your change has no call to it



kcms/lookandfeel/kcm.cpp (lines 768 - 771)
<https://git.reviewboard.kde.org/r/129872/#comment68286>

    what is this method for, your change nowhere calls it?


- Martin Gräßlin


On Jan. 28, 2017, 8:35 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129872/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2017, 8:35 a.m.)
> 
> 
> Review request for Plasma, Kai Uwe Broulik, Hugo Pereira Da Costa, and Marco Martin.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> ^^
> 
> 
> Diffs
> -----
> 
>   kcms/lookandfeel/autotests/kcmtest.cpp 29d6ec0f 
>   kcms/lookandfeel/autotests/lookandfeel/contents/defaults ef3165de 
>   kcms/lookandfeel/kcm.h c27b7f3d 
>   kcms/lookandfeel/kcm.cpp 6ea3d050 
> 
> Diff: https://git.reviewboard.kde.org/r/129872/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170128/6c521b92/attachment-0001.html>


More information about the Plasma-devel mailing list