<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/113207/">http://git.reviewboard.kde.org/r/113207/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 12th, 2013, 7:20 p.m. UTC, <b>Sebastian Kügler</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;">Code looks pretty good, there's a bunch of nitpicks, but mostly minor stuff.
What I'm not happy about is the amount of -- apparently -- copied code, the shell package and the containment. If we need those things, plasmate should probably just depend on the respective repos. Copying code is just a huge maintainance burden.
Nevertheless, I think we can merge this already.</pre>
</blockquote>
<p>On October 15th, 2013, 6:02 p.m. UTC, <b>Giorgos Tsiapaliokas</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;">> What I'm not happy about is the amount of -- apparently -- copied code, the shell package and the containment. If we need those things, plasmate should probably just depend on the respective >repos. Copying code is just a huge maintainance burden.
I am not happy either with the duplication of code, but for the time being we don't have a complete plasmoidviewer so we aren't sure 100% about
the final implementation. So I guess its ok for the moment to keep those copies(which are slightly different from the originals).
My original idea was
1. lets make something which is working
2. lets see if we can reduce/remove the duplicated code
3. open another review and lets see together if its ok
IMO having duplicated code is one of the worst things that we could ever had and I really want to avoid it but
I believe that this is not the moment that we sit down and see how can we remove this code.</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;">having duplicated code is the worst thing... after having a bad quality library :p
so between the two evils, for now duplication it is ;)</pre>
<br />
<p>- Marco</p>
<br />
<p>On October 11th, 2013, 6:40 p.m. UTC, Antonis 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 Antonis Tsiapaliokas.</div>
<p style="color: grey;"><i>Updated Oct. 11, 2013, 6:40 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasmate
</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;">The plasmoidviewer2 branch contains the initial support for plasmoidviewer2.
Right now the plasmoidviewer is able to load the applet, setup the location and the formfactor.</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>CMakeLists.txt <span style="color: grey">(2b50b09)</span></li>
<li>plasmoidviewer/CMakeLists.txt <span style="color: grey">(d36fddb)</span></li>
<li>plasmoidviewer/main.cpp <span style="color: grey">(f8a6b32)</span></li>
<li>plasmoidviewer/qmlpackages/containment/Messages.sh <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/containment/contents/code/LayoutManager.js <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/containment/contents/config/main.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/containment/contents/ui/AppletAppearance.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/containment/contents/ui/BusyOverlay.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/containment/contents/ui/main.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/containment/metadata.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/Messages.sh <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/applet/AppletError.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/applet/CompactApplet.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/applet/DefaultCompactRepresentation.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/configuration/AppletConfiguration.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/configuration/ConfigCategoryDelegate.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/configuration/ConfigurationShortcuts.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/configuration/MouseEventInputButton.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/defaults <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/layout.js <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/loader.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/contents/views/Desktop.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/qmlpackages/shell/metadata.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/view.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plasmoidviewer/view.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113207/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>