<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="https://git.reviewboard.kde.org/r/118061/">https://git.reviewboard.kde.org/r/118061/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 10th, 2014, 12:15 a.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;">Wouldn't it be better to let the Desktop settle down a bit before we start to fork things out? Actually we should find ways to share code and not having to actually fork these, which is really counter-productive.</pre>
</blockquote>
<p>On May 10th, 2014, 9:01 a.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;">That's the whole point of having shell packages. One thing that may be improved in the future is introducing a fallback mechanism between packages to not have to copy all, but i'm quite on the fence to that, and i wouldn't do it if not after it has to be provn really, really necessary.
And without attempts now I'll never really know for sure what is missing in the whole mechanism.
Furthermore, the gsoc on active is *now* and not in a few months time, the mediacenter gscoc is *now* and not in a few months time.</pre>
</blockquote>
<p>On May 12th, 2014, 3:43 a.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;">Well, we already have a buggy fork in plasmate. I've seen problems that I've fixed to plasma-shell appearing over there. I don't see why Active is going to be different.</pre>
</blockquote>
<p>On May 12th, 2014, 3:47 a.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;">FWIW, we don't need to inherit from different shells, we can actually share code through QML components too.</pre>
</blockquote>
<p>On May 12th, 2014, 10:34 a.m. UTC, <b>Bhushan Shah</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;">Well, I think both plasmate and active/mediacenter cases are totally different.. Plasmoidviewer shares similar code with desktop shell but I don't think active/mediacenter/<any other shell> will share code with the desktop.</pre>
</blockquote>
<p>On May 12th, 2014, 10:58 a.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;">Alright, then maybe I'm being overprotective. I understand plasmoidviewer is a special case (although I believe it shouldn't be forking plasma-shell code).
Either way, if this is the way it's going to be it would be good to remind how important it is to share code.
As for the review, consider my comment discarded.</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;">yep, that's true, it's very important, and thanks for reminding :)
anyways, if sharing is done, i would prefer inheritance instead of components</pre>
<br />
<p>- Marco</p>
<br />
<p>On May 10th, 2014, 4:25 p.m. UTC, Antonis Tsiapaliokas wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 May 10, 2014, 4:25 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-mobile
</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;">Add a copy of desktop shell package but remove the stuff that we
don't need. Like the contextmenu, panelconfiguration and the toolbox
</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">(c7e3797)</span></li>
<li>activeshellpackage/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/ActivityBrowser.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/ActivityCreationDialog.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/ActivityDeletionDialog.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/ActivityItem.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/ActivityList.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/ActivityManager.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/ControlButton.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/Heading.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/StoppedActivityItem.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/activitymanager/WindowPreview.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/applet/AppletError.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/applet/CompactApplet.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/applet/DefaultCompactRepresentation.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/configuration/AppletConfiguration.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/configuration/ConfigCategoryDelegate.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/configuration/ConfigurationContainmentActions.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/configuration/ConfigurationContainmentAppearance.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/configuration/ConfigurationShortcuts.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/configuration/ContainmentConfiguration.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/configuration/MouseEventInputButton.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/defaults <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/explorer/AppletDelegate.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/explorer/Tooltip.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/explorer/WidgetExplorer.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/layout.js <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/loader.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/views/Desktop.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/contents/views/Panel.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeshellpackage/package/metadata.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>qmlpackages/CMakeLists.txt <span style="color: grey">(d277441)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118061/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>