> 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