Review Request 110081: Launching amarok with --cdplay adds CD in playlist

2013-04-18 Thread Tatjana Gornak
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110081/ --- Review request for Amarok. Description --- Main problem was that addi

Re: Review Request 110070: Cleaned up playlist multilevel sorting algorithm. Added album artist special case to playlist sorting.

2013-04-18 Thread Konrad Zemek
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110070/ --- (Updated April 19, 2013, 12:08 a.m.) Review request for Amarok. Summary

Re: Review Request 110070: Introduced helper function to reduce duplication in SortScheme

2013-04-18 Thread Konrad Zemek
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110070/ --- (Updated April 19, 2013, 12:05 a.m.) Review request for Amarok. Changes

Re: Review Request 110070: Indented case statements

2013-04-18 Thread Konrad Zemek
> On April 18, 2013, 12:30 p.m., Matěj Laitl wrote: > > Thanks for the investigation and the patch, it looks very good and contains > > very welcome cleanups. Also kudos for including & cleaning up the test > > case. As I'm not really proficient in this part of Amarok code, let's give > > othe

Re: Review Request 110070: Indented case statements

2013-04-18 Thread Konrad Zemek
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110070/ --- (Updated April 18, 2013, 11:17 p.m.) Review request for Amarok. Summary

Re: Review Request 109781: Bug #312407 - don't transcode from mp3 to mp3

2013-04-18 Thread Anmol Ahuja
> On April 14, 2013, 8:23 p.m., Matěj Laitl wrote: > > Good direction, although please see suggestions below to make the patch > > perfect. Also please adapt Transcoding::SelectConfigWidget::fillInChoices() > > to cope with new Transcoding::Configuration::prettyName() semantics and to > > show

Re: Review Request 109781: Bug #312407 - don't transcode from mp3 to mp3

2013-04-18 Thread Anmol Ahuja
> On April 14, 2013, 8:23 p.m., Matěj Laitl wrote: > > Good direction, although please see suggestions below to make the patch > > perfect. Also please adapt Transcoding::SelectConfigWidget::fillInChoices() > > to cope with new Transcoding::Configuration::prettyName() semantics and to > > show

Re: Review Request 109781: Bug #312407 - don't transcode from mp3 to mp3

2013-04-18 Thread Anmol Ahuja
> On April 14, 2013, 8:23 p.m., Matěj Laitl wrote: > > src/core/collections/CollectionLocation.cpp, lines 338-345 > > > > > > I think that just removing the configuration.isValid() from the > > condition would ha

Re: Review Request 109781: Bug #312407 - don't transcode from mp3 to mp3

2013-04-18 Thread Anmol Ahuja
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109781/ --- (Updated April 18, 2013, 10:35 p.m.) Review request for Amarok. Descript

Re: Review Request 109817: JJ 313649 - No warning if there are no permissions to read file

2013-04-18 Thread Anmol Ahuja
> On April 14, 2013, 6 p.m., Matěj Laitl wrote: > > src/playlist/PlaylistModel.cpp, line 254 > > > > > > I think you should use a HTML snippet compatible with HTMLLine() (i.e. > > wrapped inside Actual text) It

Re: Review Request 109817: JJ 313649 - No warning if there are no permissions to read file

2013-04-18 Thread Anmol Ahuja
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109817/ --- (Updated April 18, 2013, 8:48 p.m.) Review request for Amarok. Descripti

Re: Review Request 110036: WIP - Simple equalizer scripting

2013-04-18 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110036/#review31242 --- Hi, first thanks for your work. The reviewboard however says fo

Re: Review Request 110070: Added album artist special case to playlist sorting. Fixed a logic bug.

2013-04-18 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110070/#review31241 --- Thanks for the investigation and the patch, it looks very good

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-18 Thread Matěj Laitl
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105290/#review31239 --- Looks very good, I'm eager to test & merge this once it is done

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-18 Thread Matěj Laitl
> On April 15, 2013, 10:47 p.m., Matěj Laitl wrote: > > src/dialogs/MusicBrainzTagger.cpp, line 87 > > > > > > I'd add > > toolBar->setToolButtonStyle( Qt::ToolButtonTextBesideIcon ); > > > > so that i

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-18 Thread Matěj Laitl
> On April 15, 2013, 12:54 p.m., Alberto Villa wrote: > > src/musicbrainz/MusicBrainzTags.cpp, line 588 > > > > > > Is this too hacky? > > Matěj Laitl wrote: > Calling sort( ... Qt::Descending_or_how_it_is_ca

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-18 Thread Alberto Villa
> On April 15, 2013, 10:47 p.m., Matěj Laitl wrote: > > src/musicbrainz/MusicBrainzTags.h, line 124 > > > > > > I'd name this: resetAllChoices() I used clearChoices() as it's more correct imho. > On April 15, 2

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-18 Thread Alberto Villa
> On April 15, 2013, 12:54 p.m., Alberto Villa wrote: > > src/musicbrainz/MusicBrainzTags.cpp, line 588 > > > > > > Is this too hacky? > > Matěj Laitl wrote: > Calling sort( ... Qt::Descending_or_how_it_is_ca

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-18 Thread Alberto Villa
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105290/ --- (Updated April 18, 2013, 10:13 a.m.) Review request for Amarok and Sergey

Re: Review Request 110036: WIP - Simple equalizer scripting

2013-04-18 Thread Ryan McCoskrie
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110036/ --- (Updated April 18, 2013, 8:03 a.m.) Review request for Amarok. Descripti

Re: Review Request 110036: WIP - Simple equalizer scripting

2013-04-18 Thread Ryan McCoskrie
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110036/ --- (Updated April 18, 2013, 7:50 a.m.) Review request for Amarok. Descripti