Review Request 120329: PlasmaShell and PlasmaSurface interfaces

[Pier Luigi Fiorini]

> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/plasma_surface.h, lines 29-31
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314687#file314687line29>
> >
> >     why wrapped in QT_BEGIN_NAMESPACE? I have never seen that in KDE code

Qt can be built with a custom namespace, that macro should take care of that.


> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/plasma_surface.h, line 125
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314687#file314687line125>
> >
> >     I'm not sure whether we need this. Plasma is using kscreen and not QScreen.

There are other components to be ported that doesn't use KScreen, for instance ksplashqml.
Besides without QScreen how can we know the wl_output?
Also, using the QScreen or Wayland backends on Plasma should allow us to map KScreen::Output to QScreen (if I recall plasmashell already do that).


> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/plasma_shell.h, line 162
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314685#file314685line162>
> >
> >     why is this in PlasmaShell? Shouldn't it be in the PlasmaSurface?

This was meant to be used for any surface should we need to move it to arbitrary global coords.


> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/plasma_shell.h, line 25
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314685#file314685line25>
> >
> >     why the QtCore/ prefix?

force of habit, guess that on Qt is often done this way plus this seems the correct way for KDE too: https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes


> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/CMakeLists.txt, lines 34-53
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314684#file314684line34>
> >
> >     what's the state of getting this into ECM?

would be ready if it wasn't ask to turn move it into find_package :)
i will take a look this evening


- Pier Luigi


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


On Sept. 23, 2014, 5:39 a.m., Pier Luigi Fiorini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120329/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2014, 5:39 a.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: kwayland
> 
> 
> Description
> -------
> 
> PlasmaShell and PlasmaSurface interfaces
> 
> 
> Diffs
> -----
> 
>   autotests/client/test_wayland_registry.cpp 54aa9a560153d00924d4e73c75f029ed1d1ad788 
>   src/client/CMakeLists.txt e00f4573ad22efc9b5776b5ef900854c04f8afd6 
>   src/client/plasma_shell.h PRE-CREATION 
>   src/client/plasma_shell.cpp PRE-CREATION 
>   src/client/plasma_surface.h PRE-CREATION 
>   src/client/plasma_surface.cpp PRE-CREATION 
>   src/client/registry.h 103be0aec9cae6d76c62fd32481eaaafa5a161f0 
>   src/client/registry.cpp 17d738415e395fb638751ac6429d1fc0e3ededd9 
> 
> Diff: https://git.reviewboard.kde.org/r/120329/diff/
> 
> 
> Testing
> -------
> 
> Work in progress Plasma port to Wayland.
> 
> 
> Thanks,
> 
> Pier Luigi Fiorini
> 
>

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