----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review46685 -----------------------------------------------------------
Thanks for the patch, it looks good. When at it, I have some improvement suggestions, please incorporate them. src/widgets/MetaQueryWidget.cpp <https://git.reviewboard.kde.org/r/114752/#comment33327> I suggest we use 24:00:00 (24 hours) as a maximum instead of 2:59:59, it cannot hurt. I also suggest to add static const int maxHours = 24; near to top of the cpp file and than use it at appropriate places. This improves maintainability. src/widgets/MetaQueryWidget.cpp <https://git.reviewboard.kde.org/r/114752/#comment33328> I suggest to create local variable for the display format for better maintainability. - Matěj Laitl On Dec. 31, 2013, 11:48 a.m., Abhay Sombanshi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114752/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2013, 11:48 a.m.) > > > Review request for Amarok. > > > Bugs: 291400 > https://bugs.kde.org/show_bug.cgi?id=291400 > > > Repository: amarok > > > Description > ------- > > CollectionBrowser filter dialog will now allow a length of 2:59:59 hours. > > > Diffs > ----- > > src/widgets/MetaQueryWidget.cpp 58601cc > > Diff: https://git.reviewboard.kde.org/r/114752/diff/ > > > Testing > ------- > > works as expected. > > > Thanks, > > Abhay Sombanshi > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel