<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 August 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>
<p>On August 25th, 2014, 2:12 p.m. UTC, <b>Marco Martin</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;">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>
</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;">I actually tried astyle but the command line options have changed yet again and the various incantations I tried made more of a mess as it made more changes than desired. If someone else can make it "go" though, that'd be great as I would very much appreciate a more consistent code base to work off of.</p></pre>
<br />
<p>- Aaron J.</p>
<br />
<p>On August 25th, 2014, 2:50 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 Aug. 25, 2014, 2:50 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/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>
<li>shell/desktopview.cpp <span style="color: grey">(0f8bde7af9bae1a17ff42d54c9882fbddc695694)</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>