<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119929/">https://git.reviewboard.kde.org/r/119929/</a>
</td>
</tr>
</table>
<br />
<p>On agosto 25th, 2014, 2:01 p.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can we maybe run a script or something after this review? It's already hard enough to decode the patch...</p></pre>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">maybe after this is merged could be tried to run astyle from<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Artistic_Style_.28astyle.29_automatic_code_formatting<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
?</p></pre>
<br />
<p>- Marco</p>
<br />
<p>On agosto 25th, 2014, 1:24 p.m. UTC, Aaron J. Seigo wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Aaron J. Seigo.</div>
<p style="color: grey;"><i>Updated Ago. 25, 2014, 1:24 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">While reviewing the plasma-shell binary for use in a device project, I came across a number of issues which are fixed in a longer-than-probably-desirable series of patches in the clean_shellcorona branch of plasma-workspace. There are two types of changes:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">improving multiscreen code; in particular there was a recursive function that would (I assume innadvertently) migrate panels to the last screen when detached. there was also potentially unecessary signal emissions, </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">code cleanups</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The code cleanups come in a few flavors:<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> getting code in line with the kdelibs coding style this project purportedly follows<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
</em> removal of dead code<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> checking of parameters where sensible<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
</em> making debug code only available in debug builds (c.f. CHECK_SCREEN_INVARIANTS; this would previously result in a bunch of compiler notices when built in release mode)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It is probably easier to review the patches in the branch than the monster review this has become, however many of the patches (though admittedly not all) are built one atop the other making individual review tricky; it was one of those "tug on a loose thread, and the whole sweater came unravelled" type things.</p></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>shell/desktopview.cpp <span style="color: grey">(0f8bde7af9bae1a17ff42d54c9882fbddc695694)</span></li>
<li>shell/scripting/scriptengine.cpp <span style="color: grey">(94e93fe810edb1743b84cdca168bc7f7e556e982)</span></li>
<li>shell/shellcorona.h <span style="color: grey">(ebfbdc7ceea9f6cfb387c1931e78faa9e43a894d)</span></li>
<li>shell/shellcorona.cpp <span style="color: grey">(29935a51ffc490e1d7e91e2a1109764a4ff2d101)</span></li>
<li>shell/widgetexplorer/widgetexplorer.cpp <span style="color: grey">(0ad5cc8519c8f37ce20e49332491ea0469ee7a29)</span></li>
<li>shell/containmentconfigview.cpp <span style="color: grey">(f489f1771f3de0a0a5fb7e385615e5981b2f240f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119929/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>