[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Move cascadeDesktop and unclutterDesktop to Placement
From: Thomas_Lübking <thomas.luebking () web ! de>
Date: 2012-11-22 11:56:24
Message-ID: 20121122115624.27840.34970 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107416/#review22374
-----------------------------------------------------------
Ship it!
I would have said that it's because this way the function needs to perform =
an upstream access to the workspace, get the clientlist etc, but that happe=
ned before anyway.
Given the singleton character of workspace and placement it's more like a n=
ested namespace anyway.
- Thomas L=C3=BCbking
On Nov. 22, 2012, 10:10 a.m., Martin Gr=C3=A4=C3=9Flin wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107416/
> -----------------------------------------------------------
> =
> (Updated Nov. 22, 2012, 10:10 a.m.)
> =
> =
> Review request for kwin.
> =
> =
> Description
> -------
> =
> Move cascadeDesktop and unclutterDesktop to Placement
> =
> It is more Placement related and does not really fit into geometry given
> that it only calls methods on Placement. It probably only was inside
> Workspace due to being part of the DBus interface. The DBus methods are
> used by external components so it needs to stay.
> =
> The DBus wrapper is calling the methods directly. Therefore the Placment
> is provided by Workspace. It's worth thinking about making Placement a
> singleton, given that only one object is created.
> =
> =
> Diffs
> -----
> =
> kwin/dbusinterface.cpp 6d64a9fc6acd60cba212454586a30a8e34ff296e =
> kwin/geometry.cpp ef63cae95841c5c80d2a00430f79812d6dc95160 =
> kwin/placement.h 0fe894ef77ea95b88133b2911285a04cf7a0175e =
> kwin/placement.cpp d5ad358016dd3bfbd64ae9b9a13a952cc357b2fe =
> kwin/workspace.h 708e23333dbce69bc4cf7e808cf752ceb701d8d1 =
> =
> Diff: http://git.reviewboard.kde.org/r/107416/diff/
> =
> =
> Testing
> -------
> =
> it compiles :-)
> =
> =
> Thanks,
> =
> Martin Gr=C3=A4=C3=9Flin
> =
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/107416/">http://git.reviewboard.kde.org/r/107416/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I would have said that \
it's because this way the function needs to perform an upstream access to the \
workspace, get the clientlist etc, but that happened before anyway. Given the \
singleton character of workspace and placement it's more like a nested namespace \
anyway.</pre> <br />
<p>- Thomas</p>
<br />
<p>On November 22nd, 2012, 10:10 a.m., Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for kwin.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Nov. 22, 2012, 10:10 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Move cascadeDesktop and unclutterDesktop to Placement
It is more Placement related and does not really fit into geometry given
that it only calls methods on Placement. It probably only was inside
Workspace due to being part of the DBus interface. The DBus methods are
used by external components so it needs to stay.
The DBus wrapper is calling the methods directly. Therefore the Placment
is provided by Workspace. It's worth thinking about making Placement a
singleton, given that only one object is created.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">it compiles :-)</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kwin/dbusinterface.cpp <span style="color: \
grey">(6d64a9fc6acd60cba212454586a30a8e34ff296e)</span></li>
<li>kwin/geometry.cpp <span style="color: \
grey">(ef63cae95841c5c80d2a00430f79812d6dc95160)</span></li>
<li>kwin/placement.h <span style="color: \
grey">(0fe894ef77ea95b88133b2911285a04cf7a0175e)</span></li>
<li>kwin/placement.cpp <span style="color: \
grey">(d5ad358016dd3bfbd64ae9b9a13a952cc357b2fe)</span></li>
<li>kwin/workspace.h <span style="color: \
grey">(708e23333dbce69bc4cf7e808cf752ceb701d8d1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107416/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic