Review Request 113207: Plasmate: Request for merge, branch plasmoidviewer2 to frameworks-scratch

[Marco Martin]

> On Oct. 12, 2013, 7:20 p.m., Sebastian Kügler wrote:
> > 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.
> 
> Giorgos Tsiapaliokas wrote:
>     > 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.

having duplicated code is the worst thing... after having a bad quality library :p

so between the two evils, for now duplication it is ;)


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113207/#review41613
-----------------------------------------------------------


On Oct. 11, 2013, 6:40 p.m., Antonis Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113207/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2013, 6:40 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasmate
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2b50b09 
>   plasmoidviewer/CMakeLists.txt d36fddb 
>   plasmoidviewer/main.cpp f8a6b32 
>   plasmoidviewer/qmlpackages/containment/Messages.sh PRE-CREATION 
>   plasmoidviewer/qmlpackages/containment/contents/code/LayoutManager.js PRE-CREATION 
>   plasmoidviewer/qmlpackages/containment/contents/config/main.xml PRE-CREATION 
>   plasmoidviewer/qmlpackages/containment/contents/ui/AppletAppearance.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/containment/contents/ui/BusyOverlay.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/containment/contents/ui/main.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/containment/metadata.desktop PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/Messages.sh PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/applet/AppletError.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/applet/CompactApplet.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/applet/DefaultCompactRepresentation.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/configuration/AppletConfiguration.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/configuration/ConfigCategoryDelegate.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/configuration/ConfigurationShortcuts.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/configuration/MouseEventInputButton.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/defaults PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/layout.js PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/loader.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/views/Desktop.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/metadata.desktop PRE-CREATION 
>   plasmoidviewer/view.h PRE-CREATION 
>   plasmoidviewer/view.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/113207/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Antonis Tsiapaliokas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20131016/42aa38ae/attachment.html>