> 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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to