<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/112447/">http://git.reviewboard.kde.org/r/112447/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 2nd, 2013, 2:40 p.m. UTC, <b>Aaron J. Seigo</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;">the Plasma/Shell package definition should live in libplasma along with the others. this will remove the need for the PluginLoader subclass and ensure that Corona can be used by any application that simply links to libplasma, which is obviously a requirement.
the other classes should perhaps find a more appropriate home as well rather than creating a whole new library. really, this is not the "Plasma View Library" but the "if you are writing a QML shell with libplasma you will find these couple of classes useful..." library. most optimal would be if this could be tucked away with the rest of the QML support for plasma, perhaps in the script engine library itself.
the config classes also scream out to be made internal and not public API. as the main class used is a QQuickView, putting this in the QML support would be sensible and then all of this could be nicely hidden away.
the classes also seem to be in need of some API review before we commit to binary compatibility for them ... something for one of the upcoming monday meetings?</pre>
</blockquote>
</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;">in master most of issues should be addressed now</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 2nd, 2013, 2:40 p.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/112447/diff/1/?file=186293#file186293line169" style="color: black; font-weight: bold; text-decoration: underline;">src/plasmaview/configview.h</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">169</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">//</span><span class="cs">TODO</span><span class="c1">: the config view for the containment should be a subclass</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this TODO seems to be done</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">confirmed is a done todo</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 2nd, 2013, 2:40 p.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/112447/diff/1/?file=186293#file186293line170" style="color: black; font-weight: bold; text-decoration: underline;">src/plasmaview/configview.h</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">170</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">//</span><span class="cs">TODO</span><span class="c1">: is it possible to move this in the shell?</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">is this TODO still valid given that this code is moving into a library?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">confirmed is a done todo</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 2nd, 2013, 2:40 p.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/112447/diff/1/?file=186295#file186295line34" style="color: black; font-weight: bold; text-decoration: underline;">src/plasmaview/containmentconfigview_p.h</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">34</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">//</span><span class="cs">TODO</span><span class="c1">: is it possible to move this in the shell?</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">another stray TODO?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">confirmed is a done todo</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 2nd, 2013, 2:40 p.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/112447/diff/1/?file=186296#file186296line51" style="color: black; font-weight: bold; text-decoration: underline;">src/plasmaview/containmentconfigview_p.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">51</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Plasma</span><span class="o">::</span><span class="n">Package</span> <span class="n">pkg</span> <span class="o">=</span> <span class="n">Plasma</span><span class="o">::</span><span class="n">PluginLoader</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span class="o">-></span><span class="n">loadPackage</span><span class="p">(</span><span class="s">"Plasma/Generic"</span><span class="p">);</span></pre></td>
</tr>
<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">52</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">pkg</span><span class="p">.</span><span class="n">setDefaultPackageRoot</span><span class="p">(</span><span class="s">"plasma/wallpapers"</span><span class="p">);</span></pre></td>
</tr>
<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">53</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">pkg</span><span class="p">.</span><span class="n">setPath</span><span class="p">(</span><span class="n">m_containment</span><span class="o">-></span><span class="n">wallpaper</span><span class="p">());</span></pre></td>
</tr>
<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">54</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QFile</span> <span class="nf">file</span><span class="p">(</span><span class="n">pkg</span><span class="p">.</span><span class="n">filePath</span><span class="p">(</span><span class="s">"config"</span><span class="p">,</span> <span class="s">"main.xml"</span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">why is this using Plasma/Generic instead of a Plasma/QmlWallpaper (or whatever) package?
sprinkling our code with setDefaultPackageRoot and filePath calls with literal values defeats the purpose of Plasma::Package: to encapsulate those details.
if we were to move where the wallpaper packages were kept, then what? we search every single bit of code that might use wallpapers?
no, that's why we have Plasma::Package subclasses! there should be a package type for these wallpapers.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I now made a QmlWallpaper packagestructure locally in the shell, now it uses that (it pretty much duplicates Generic)</pre>
<br />
<p>- Marco</p>
<br />
<p>On September 2nd, 2013, 12:53 p.m. UTC, Giorgos Tsiapaliokas 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 Plasma.</div>
<div>By Giorgos Tsiapaliokas.</div>
<p style="color: grey;"><i>Updated Sept. 2, 2013, 12:53 p.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;">This patch creates a new library out of the shell dir.
We need this library in order to implement plasmoidviewer 2.0</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;">The code is locate at git@git.kde.org:clones/plasma-framework/tsiapaliwkas/pf5.git .
how to test it
* git clone git@git.kde.org:clones/plasma-framework/tsiapaliwkas/pf5.git
* cd pf5
* git checkout split7
* build it and install it
* use plasma-shell
I haven't noticed any issues.</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>src/CMakeLists.txt <span style="color: grey">(281d146)</span></li>
<li>src/plasmaview/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/PlasmaViewConfig.cmake.in <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/configview.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/configview.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/containmentconfigview_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/containmentconfigview_p.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/currentcontainmentactionsmodel_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/currentcontainmentactionsmodel_p.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/includes/PlasmaView/ConfigView <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/includes/PlasmaView/ContainmentConfigView <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/includes/PlasmaView/ShellPluginLoader <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/includes/PlasmaView/View <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/shellpackage_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/shellpackage_p.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/shellpluginloader.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/shellpluginloader.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/view.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plasmaview/view.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/shell/CMakeLists.txt <span style="color: grey">(3da019f)</span></li>
<li>src/shell/configview.h <span style="color: grey">(2e8f68f)</span></li>
<li>src/shell/configview.cpp <span style="color: grey">(fea5a73)</span></li>
<li>src/shell/containmentconfigview.h <span style="color: grey">(619fa14)</span></li>
<li>src/shell/containmentconfigview.cpp <span style="color: grey">(235a33f)</span></li>
<li>src/shell/currentcontainmentactionsmodel.h <span style="color: grey">(db94da1)</span></li>
<li>src/shell/currentcontainmentactionsmodel.cpp <span style="color: grey">(c955bef)</span></li>
<li>src/shell/shellcorona.cpp <span style="color: grey">(ffdbfe8)</span></li>
<li>src/shell/shellpackage.h <span style="color: grey">(99dc460)</span></li>
<li>src/shell/shellpackage.cpp <span style="color: grey">(74aea5c)</span></li>
<li>src/shell/shellpluginloader.h <span style="color: grey">(1d3dade)</span></li>
<li>src/shell/shellpluginloader.cpp <span style="color: grey">(8b2e1dd)</span></li>
<li>src/shell/view.h <span style="color: grey">(7e6b2d9)</span></li>
<li>src/shell/view.cpp <span style="color: grey">(a0c6168)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112447/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>