----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109019/#review27994 -----------------------------------------------------------
Almost there! :-) kdeui/shortcuts/kglobalaccel.h <http://git.reviewboard.kde.org/r/109019/#comment20953> Please remove the extra space at the end of this line. kdeui/shortcuts/kglobalaccel.h <http://git.reviewboard.kde.org/r/109019/#comment20954> Please name the parameters and document them. kdeui/shortcuts/kglobalaccel.cpp <http://git.reviewboard.kde.org/r/109019/#comment20966> Wouldn't it be better to use actionShortcuts here? At that point, you could know about an action because of a default shortcut but it could currently have no shortcut. I'd expect shortcut() and hasShortcut() to be consistent together, but right now they could be out of sync. Beside it would help you get rid of the const_cast which can't stay. Actually I think it could be a simple: return !shortcut(action).toList().isEmpty(); - Kevin Ottens On Feb. 22, 2013, 10:59 p.m., Valentin Rusu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109019/ > ----------------------------------------------------------- > > (Updated Feb. 22, 2013, 10:59 p.m.) > > > Review request for KDE Frameworks, David Faure and Kevin Ottens. > > > Description > ------- > > KDELibs cleanup task: move global shortcut facilities from KAction to > KGlobalAccel. The second step, registering QActions instead of KActions, will > be done after completing this review. > > > Diffs > ----- > > KDE5PORTING.html e1b41d1 > kdeui/actions/kaction.h ddaa4de > kdeui/actions/kaction.cpp ec63a72 > kdeui/actions/kaction_p.h b50e678 > kdeui/shortcuts/kglobalaccel.h ed68bba > kdeui/shortcuts/kglobalaccel.cpp 36dbab7 > kdeui/shortcuts/kglobalaccel_p.h 04feaba > > Diff: http://git.reviewboard.kde.org/r/109019/diff/ > > > Testing > ------- > > Code compiles > > > Thanks, > > Valentin Rusu > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
