Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated May 6, 2014, 4:02 p.m.) Status -- This change has been marked as submitted. Review request for Amarok and Yuri Chornoivan. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review57426 --- This review has been submitted with commit 01d512aa1168c8dffdf8942026563be164053116 by Yuri Chornoivan on behalf of Abhay Sombanshi to branch master. - Commit Hook On Jan. 5, 2014, 2:17 p.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 2:17 p.m.) Review request for Amarok and Yuri Chornoivan. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review57368 --- Has this been commited? Yuri can you commit it? Want me to commit it? - Albert Astals Cid On Jan. 5, 2014, 2:17 p.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 2:17 p.m.) Review request for Amarok and Yuri Chornoivan. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
On May 5, 2014, 10:58 p.m., Albert Astals Cid wrote: Has this been commited? Yuri can you commit it? Want me to commit it? I just wanted that it to be committed by the team leader (Matěj). Please commit the patch if you find this appropriate. ;) - Yuri --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review57368 --- On Jan. 5, 2014, 2:17 p.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 2:17 p.m.) Review request for Amarok and Yuri Chornoivan. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review47218 --- Ship it! Ship It! - Yuri Chornoivan On Jan. 5, 2014, 2:17 p.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 2:17 p.m.) Review request for Amarok and Yuri Chornoivan. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 11:25 a.m.) Review request for Amarok and Yuri Chornoivan. Changes --- Add Yuri to people. Yuri, do you know whether there is a better way of localizing QTimeEdit? (in MetaQueryWidget::makeLengthSelection()) Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review46844 --- I see, this patch is almost ready, please wait for Yuri to answer and then fix remaining problems. Also please don't forget about: 1. Add commit tags to the description - see quickgit.kde.org/?p=kdelibs.gita=blobf=.commit-template - namely: BUG: , FIXED-IN: 2.9, REVIEW: ### - so that they end up in the commit - also please update the description to be current src/widgets/MetaQueryWidget.cpp https://git.reviewboard.kde.org/r/114752/#comment33442 So the maximum time is 23:59:59 - this calculation is wrong. src/widgets/MetaQueryWidget.cpp https://git.reviewboard.kde.org/r/114752/#comment33443 Please: 1. provide context for the string - use i18nc() call with context like time format for specifying track length - hours, minutes, seconds 2. respect Amarok coding style - spaces around arguments - Matěj Laitl On Jan. 5, 2014, 11:25 a.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 11:25 a.m.) Review request for Amarok and Yuri Chornoivan. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review46845 --- Ship it! Ship It! - Yuri Chornoivan On Jan. 5, 2014, 10:25 a.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 10:25 a.m.) Review request for Amarok and Yuri Chornoivan. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 5, 2014, 7:47 p.m.) Review request for Amarok and Yuri Chornoivan. Changes --- *Display format is localized with context. Bugs: 291400 https://bugs.kde.org/show_bug.cgi?id=291400 Repository: amarok Description --- CollectionBrowser filter dialog will now allow a length of 24 hours. Diffs (updated) - ChangeLog 0eb03c8 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review46778 --- Looks better, please see another round of suggestions below and also: 1. Add commit tags to the description - see quickgit.kde.org/?p=kdelibs.gita=blobf=.commit-template - namely: BUG: , FIXED-IN: 2.9, REVIEW: ### - so that they end up in the commit - also please update the description to be current 2. Add a line to the ChangeLog file (top-level in the repo) - under BUGFIXES on top. src/widgets/MetaQueryWidget.cpp https://git.reviewboard.kde.org/r/114752/#comment33409 Another thing on my mind: it may have sense to have the format localized - please see KLocale class http://api.kde.org/4.x-api/kdelibs-apidocs/kdecore/html/classKLocale.html and perhaps ask on #kde-i18n IRC channel on Freenode. src/widgets/MetaQueryWidget.cpp https://git.reviewboard.kde.org/r/114752/#comment33408 I see no point in allowing only 23:59:59, just make it 24:00:00 please, unless there is a usability problem with it. (and please update other places to match) - Matěj Laitl On Jan. 3, 2014, 6:33 p.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 3, 2014, 6:33 p.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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
On Jan. 4, 2014, 9:28 p.m., Matěj Laitl wrote: src/widgets/MetaQueryWidget.cpp, line 850 https://git.reviewboard.kde.org/r/114752/diff/2/?file=229709#file229709line850 I see no point in allowing only 23:59:59, just make it 24:00:00 please, unless there is a usability problem with it. (and please update other places to match) The maximum allowed value in QTime is 23:59:59 hours. - Abhay --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/#review46778 --- On Jan. 3, 2014, 11:03 p.m., Abhay Sombanshi wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 3, 2014, 11:03 p.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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- 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
Re: Review Request 114752: BugFix : 291400 - Maximum allowed length in CollectionBrowser filter dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114752/ --- (Updated Jan. 3, 2014, 5:33 p.m.) Review request for Amarok. Changes --- maximum length is changed to 24 hours. 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 (updated) - 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
