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