----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127352/#review93470 -----------------------------------------------------------
Fix it, then Ship it! couple of nitpicking (which I cand easily fix myself in case you are busy elsewhere. asside from that, I could not test because have no working wayland setup here (too old distro/drivers), so I'll just trust you. other than that, good to go imho. (and don't hesitate to also push to oxygen, if you have time, and without a review. Note that I can do it myself too -without testing-, otherwise) kstyle/breezeshadowhelper.h (line 33) <https://git.reviewboard.kde.org/r/127352/#comment63727> I thought the policy in case of "external" dependencies, was to include the header rather than using forward declarations. (in case for instance a class is turned into a struct, a namespace into a class, or whatever). At least have been doing so with Qt since forever. If not, please add some indentation here after the braces) kstyle/breezeshadowhelper.h (line 120) <https://git.reviewboard.kde.org/r/127352/#comment63728> some comments before the new methods ? kstyle/breezeshadowhelper.h (line 134) <https://git.reviewboard.kde.org/r/127352/#comment63729> idem kstyle/breezeshadowhelper.h (line 172) <https://git.reviewboard.kde.org/r/127352/#comment63730> other class members start with "_" rather than "m_" please keep it internally consistent (even if not with the rest of e.g. kwin) kstyle/breezeshadowhelper.cpp (line 82) <https://git.reviewboard.kde.org/r/127352/#comment63731> please keep "//_________________________" (the number of "_" being unimportant) before the beginning of each function, for consistency with the rest of the class. kstyle/breezeshadowhelper.cpp (line 381) <https://git.reviewboard.kde.org/r/127352/#comment63732> //______________________________ kstyle/breezeshadowhelper.cpp (line 435) <https://git.reviewboard.kde.org/r/127352/#comment63733> //________________________________ (and elsewhere in case I missed some) - Hugo Pereira Da Costa On March 12, 2016, 8:28 a.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127352/ > ----------------------------------------------------------- > > (Updated March 12, 2016, 8:28 a.m.) > > > Review request for Plasma and Hugo Pereira Da Costa. > > > Repository: breeze > > > Description > ------- > > Integrate with Wayland and create a ShadowManager if available. > With the ShadowManager it's possible to create a Shadow for a Surface. > > The Wayland shadow is very similar to the X11 based one, so a lot of > code can be shared. The code is slightly refactored to share the common > code paths to create and destroy the shadow. > > > Diffs > ----- > > kstyle/CMakeLists.txt b2c6dc847fef1c7876a0ac1af9e28f47422b620b > kstyle/breezeshadowhelper.h 0c868d3686eec67e630ce08c36df6b4cf34d7f1c > kstyle/breezeshadowhelper.cpp 1eb1ddc6a154d8865b39936718f6a5d5ef8f7fdd > kstyle/config-breeze.h.cmake ba2b5f64c1ca855c541fd7eb0e919b8f897f481f > > Diff: https://git.reviewboard.kde.org/r/127352/diff/ > > > Testing > ------- > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel