> On April 3, 2013, 10:11 p.m., Matěj Laitl wrote: > > src/core/transcoding/TranscodingConfiguration.h, line 61 > > <http://git.reviewboard.kde.org/r/109781/diff/4/?file=127284#file127284line61> > > > > You've introduced and enum, why do you accept an int? Also please > > rename transConfig to trackSelection and don't provide default value for it. > > Anmol Ahuja wrote: > But with the default value, don't calls like: > Transcoding::Configuration configuration( Transcoding::JUST_COPY ); > seem better than > Transcoding::Configuration configuration( Transcoding::JUST_COPY, > Transcoding::Configuration::TranscodeAll );
Oh, you're right, please keep the default value then. :-) (perhaps at the other place too, if it makes sense) Also please mention somewhere in documentation that the TrackSelection is not used if the encoder is Transcoding::JUST_COPY. On the other hand, TrackSelection value *should* be used when encoder is Transcoding::INVALID (meaning ask every time) - it should serve as a default value for the radio button in the Transcode dialog (please make it so if it isn't already the case); documentation of this behaviour somewhere would be nice, too. - Matěj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109781/#review30336 ----------------------------------------------------------- On April 1, 2013, 2:46 p.m., Anmol Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109781/ > ----------------------------------------------------------- > > (Updated April 1, 2013, 2:46 p.m.) > > > Review request for Amarok. > > > Description > ------- > > Added 3 checkbox in the TranscodingAssistantDialog: > "Transcode all tracks" - transcode all tracks, the current default behavior > "Ignore files which're already the selected format" - transcode only if > source and setination file formats are different > "Transcode only when needed for playability" - transcode only when needed for > playability in the destination collection > > > Diffs > ----- > > ChangeLog 8db61e8 > src/core-impl/collections/db/sql/SqlCollectionLocation.cpp 11fa33e > src/core-impl/collections/ipodcollection/jobs/IpodCopyTracksJob.cpp 8a40c6c > src/core-impl/collections/umscollection/UmsCollectionLocation.cpp e0ba0ac > src/core/transcoding/TranscodingConfiguration.h 98b2bb8 > src/core/transcoding/TranscodingConfiguration.cpp f97a43d > src/transcoding/TranscodingAssistantDialog.h 76287a7 > src/transcoding/TranscodingAssistantDialog.cpp 6bba0ec > src/transcoding/TranscodingAssistantDialog.ui 2505bd3 > src/transcoding/TranscodingJob.h 6170c2a > src/transcoding/TranscodingJob.cpp cab76a7 > src/transcoding/TranscodingOptionsStackedWidget.h 9495d44 > src/transcoding/TranscodingOptionsStackedWidget.cpp cf4f328 > > Diff: http://git.reviewboard.kde.org/r/109781/diff/ > > > Testing > ------- > > Works as expected > All build tests passed > > > Thanks, > > Anmol Ahuja > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
