bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> taglibwritertest.cpp:74
> +
> + QTest::addRow("mp3")
> + << QStringLiteral("mp3")
Can you sort the tests alphabetically? It does not matter much, but IMHO it is
nicer to have some rule for ordering, instead of arbitrary order.
> taglibwriter.cpp:23
> QStringList types = {
> QStringLiteral("audio/mpeg"),
> QStringLiteral("audio/mpeg3"),
Can you also order these alphabetically based on the main mime type (aliases
immediate after)?
> taglibwriter.cpp:33
> + QStringLiteral("audio/x-opus+ogg"),
> + QStringLiteral("audio/wav"),
> + QStringLiteral("audio/x-aiff"),
audio/wav and the next three are not backed by tests AFAICS, can you add these
in a separate review, accompanied by tests files?
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D15704
To: astippich, mgallien, bruns
Cc: svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich,
spoorun, ngraham, bruns, abrahams
