[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request 110199: Move RootInfo and WinInfo into an own header and impl file
From: Thomas_Lübking <thomas.luebking () gmail ! com>
Date: 2013-04-26 16:12:42
Message-ID: 20130426161242.5891.32914 () 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/110199/#review31636
-----------------------------------------------------------
kwin/netinfo.cpp
<http://git.reviewboard.kde.org/r/110199/#comment23549>
Workspace::self()?
kwin/netinfo.cpp
<http://git.reviewboard.kde.org/r/110199/#comment23550>
idem
kwin/workspace.h
<http://git.reviewboard.kde.org/r/110199/#comment23551>
as it's private and RootInfo and "netinfo.h" in included in workspace.cpp - why lie about the type?
- Thomas Lübking
On April 26, 2013, 8:47 a.m., Martin Gräßlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110199/
> -----------------------------------------------------------
>
> (Updated April 26, 2013, 8:47 a.m.)
>
>
> Review request for kwin.
>
>
> Description
> -------
>
> Move RootInfo and WinInfo into an own header and impl file
>
> Main motivation for this change is that it's unhandy to have the class
> definition in workspace.h and client.h while the implementation is in
> events.cpp although nothing in events.cpp uses it directly.
>
> By getting it out of workspace.h we get the header a little bit smaller
> which should improve compile time given that it's included almost
> everywhere.
>
> In events.cpp the enum usage is changed to NETWinInfo as that's the class
> where they are defined.
>
>
> Diffs
> -----
>
> kwin/CMakeLists.txt e386e88aa82f1dbced291d98f6620fd6fdf6e960
> kwin/client.h 64609a94888782f4edc3fc48c6f9c75cb57582a8
> kwin/deleted.cpp 20258839d34f173e3c041665433739594719ee8e
> kwin/events.cpp e99558196ff968b5fbd0f15c5c6fed4ffed6aedc
> kwin/manage.cpp bef376503cdd0259dbaab75180df6e46185b3d96
> kwin/netinfo.h PRE-CREATION
> kwin/netinfo.cpp PRE-CREATION
> kwin/toplevel.h 5211e3819523cdb8b6089fbef3e1eae75daf9eff
> kwin/workspace.h 22f2f0aea2a3a52433c0fcac93c4eaf31c24fed3
> kwin/workspace.cpp 177cc11a04955658b4d578bac818aee60b25eeef
>
> Diff: http://git.reviewboard.kde.org/r/110199/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Martin Gräßlin
>
>
[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/110199/">http://git.reviewboard.kde.org/r/110199/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/110199/diff/1/?file=141118#file141118line35" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/netinfo.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">35</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">workspace</span> <span class="o">=</span> <span class="n">ws</span><span \
class="p">;</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Workspace::self()?</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/110199/diff/1/?file=141118#file141118line138" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/netinfo.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">138</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">m_client</span><span class="o">-></span><span \
class="n">workspace</span><span class="p">()</span><span class="o">-></span><span \
class="n">sendClientToDesktop</span><span class="p">(</span><span \
class="n">m_client</span><span class="p">,</span> <span class="n">desktop</span><span \
class="p">,</span> <span class="nb">true</span><span class="p">);</span></pre></td> \
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">idem</pre> \
</div> <br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/110199/diff/1/?file=141120#file141120line550" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/workspace.h</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">private:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">549</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">RootInfo</span><span class="o">*</span> <span \
class="n">rootInfo</span><span class="p">;</span></pre></td> <th bgcolor="#e9eaa8" \
style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">548</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span \
class="hl">NET</span>RootInfo</span><span class="o">*</span> <span \
class="n">rootInfo</span><span class="p">;</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">as it's \
private and RootInfo and "netinfo.h" in included in workspace.cpp - why lie \
about the type?</pre> </div>
<br />
<p>- Thomas</p>
<br />
<p>On April 26th, 2013, 8:47 a.m. UTC, Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.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 April 26, 2013, 8:47 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 RootInfo and WinInfo into an own header and impl file
Main motivation for this change is that it's unhandy to have the class
definition in workspace.h and client.h while the implementation is in
events.cpp although nothing in events.cpp uses it directly.
By getting it out of workspace.h we get the header a little bit smaller
which should improve compile time given that it's included almost
everywhere.
In events.cpp the enum usage is changed to NETWinInfo as that's the class
where they are defined.</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/CMakeLists.txt <span style="color: \
grey">(e386e88aa82f1dbced291d98f6620fd6fdf6e960)</span></li>
<li>kwin/client.h <span style="color: \
grey">(64609a94888782f4edc3fc48c6f9c75cb57582a8)</span></li>
<li>kwin/deleted.cpp <span style="color: \
grey">(20258839d34f173e3c041665433739594719ee8e)</span></li>
<li>kwin/events.cpp <span style="color: \
grey">(e99558196ff968b5fbd0f15c5c6fed4ffed6aedc)</span></li>
<li>kwin/manage.cpp <span style="color: \
grey">(bef376503cdd0259dbaab75180df6e46185b3d96)</span></li>
<li>kwin/netinfo.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/netinfo.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/toplevel.h <span style="color: \
grey">(5211e3819523cdb8b6089fbef3e1eae75daf9eff)</span></li>
<li>kwin/workspace.h <span style="color: \
grey">(22f2f0aea2a3a52433c0fcac93c4eaf31c24fed3)</span></li>
<li>kwin/workspace.cpp <span style="color: \
grey">(177cc11a04955658b4d578bac818aee60b25eeef)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110199/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