> On Sept. 24, 2013, 4:23 p.m., Sune Vuorela wrote: > > tier1/kguiaddons/src/lib/colors/kcolorutils.cpp, line 42 > > <http://git.reviewboard.kde.org/r/112816/diff/1/?file=190519#file190519line42> > > > > My initial reaction is that it could return a bool wether or not things > > went okay, given there is a 'path that does nothing' in the code. Besides > > that, everything looks great.
I wrote it this way to be consistent with QColor::get*() methods. I think users of such code are going to be used to QColor API, so it makes more sense this way, what do you think? - Aurélien ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112816/#review40692 ----------------------------------------------------------- On Sept. 24, 2013, 4:19 p.m., Aurélien Gâteau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112816/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2013, 4:19 p.m.) > > > Review request for KDE Frameworks and kdelibs. > > > Description > ------- > > This is a two-commit patch: > > 1. Add a KColorUtils::getHcy() method > > 2. In kcolorutils demo: use KColorUtils::getHcy() instead of the internal > KColorSpaces::KHCY > > This is a required first step to make kconfigwidgets build standalone > > > Diffs > ----- > > staging/kconfigwidgets/tests/kcolorutilsdemo.cpp 940569e > tier1/kguiaddons/src/lib/colors/kcolorutils.h c20c6f7 > tier1/kguiaddons/src/lib/colors/kcolorutils.cpp 9212bba > > Diff: http://git.reviewboard.kde.org/r/112816/diff/ > > > Testing > ------- > > kcolorutilsdemo still works. > > > Thanks, > > Aurélien Gâteau > >