----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110910/#review34398 -----------------------------------------------------------
Ship it! Not tested, but by the code looks good to go from my POV! Just some µ-nitpicks which could be fixed while committing, no need to review again. libs/widgets/KoResourceItemChooser.cpp <http://git.reviewboard.kde.org/r/110910/#comment25270> µ-nitpick: separate "index," and "tag" with one space not part of this patch, but while you are on it... libs/widgets/KoResourceItemChooser.cpp <http://git.reviewboard.kde.org/r/110910/#comment25271> const int count perhaps, more const is usually better :) libs/widgets/KoResourceItemChooserContextMenu.h <http://git.reviewboard.kde.org/r/110910/#comment25268> µ-nitpick: space behind "resource" ;) libs/widgets/KoResourceItemChooserContextMenu.cpp <http://git.reviewboard.kde.org/r/110910/#comment25269> Still think that the trailing ":" in "Assign to tag:" better is removed, because it only clutters and duplicates the semantics of the > symbol before the submenu. Well, IMHO. - Friedrich W. H. Kossebau On June 12, 2013, 8:01 p.m., Sascha Suelzer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110910/ > ----------------------------------------------------------- > > (Updated June 12, 2013, 8:01 p.m.) > > > Review request for Calligra. > > > Description > ------- > > Refactored context menu creation code out of KoResourceItemChooser and into > its own QMenu class, any style feedback is welcome. > Also a tiny amount of whitespace cleanup, but only for the files affected by > the code changes. > > > Diffs > ----- > > libs/widgets/KoResourceItemChooser.h 29fb09a > libs/widgets/KoResourceItemChooser.cpp 4ae16ad > libs/widgets/KoResourceItemChooserContextMenu.h a33a132 > libs/widgets/KoResourceItemChooserContextMenu.cpp 6af2fe1 > > Diff: http://git.reviewboard.kde.org/r/110910/diff/ > > > Testing > ------- > > Tested only for Krita, everything seems to work as it should. > > > Thanks, > > Sascha Suelzer > >
_______________________________________________ calligra-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/calligra-devel
