----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106633/#review19591 -----------------------------------------------------------
Nice cleanup. I found some places to niggle, but apart from that, I'm fine with this. It applies, compiles, works and is an improvement, imo. plugins/dockers/CMakeLists.txt <http://git.reviewboard.kde.org/r/106633/#comment15577> Why the change in order? Changing the order of items in CMakeLists.txt often makes for difficult merges. plugins/dockers/styledocker/StrokeFillWidget.h <http://git.reviewboard.kde.org/r/106633/#comment15581> updateWidget probably needs to be a bit more expressively named, since it is, I guess, the old updateStyle. Maybe updateWidgetStyleSettings or something like that. plugins/dockers/styledocker/StrokeFillWidget.h <http://git.reviewboard.kde.org/r/106633/#comment15578> Why all the space between type and variable name? plugins/dockers/styledocker/StrokeFillWidget.cpp <http://git.reviewboard.kde.org/r/106633/#comment15579> Er, yes, that doesn't come as a surprise? plugins/dockers/styledocker/StyleDocker.cpp <http://git.reviewboard.kde.org/r/106633/#comment15580> This looks weird, and is one of the places were if you want to push it like this I really would like a comment. - Boudewijn Rempt On Sept. 29, 2012, 2:35 p.m., Inge Wallin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106633/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2012, 2:35 p.m.) > > > Review request for Calligra. > > > Description > ------- > > This patch fulfills a very old wish of ours, namely the separation of the > stroke/fill docker into a widget and an embedding docker. This makes the > stroke/fill widget reusable in other places, e.g. a style manager for graphic > styles. > > I have tried to make the user experience exactly the same as for the old > version even though there are lots of misfeatures with it. Improving the > user experience will come as future patches, and I don't want to do that > without discussing it with other people. > > > Diffs > ----- > > plugins/dockers/CMakeLists.txt c9b4f44 > plugins/dockers/styledocker/StrokeFillWidget.h PRE-CREATION > plugins/dockers/styledocker/StrokeFillWidget.cpp PRE-CREATION > plugins/dockers/styledocker/StyleDocker.h b0301e6 > plugins/dockers/styledocker/StyleDocker.cpp 376ffef > > Diff: http://git.reviewboard.kde.org/r/106633/diff/ > > > Testing > ------- > > Extensive manual testing. > > > Thanks, > > Inge Wallin > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
