bruns added inline comments.
INLINE COMMENTS
> embeddedimagedata.cpp:67
> +
> + if (types & EmbeddedImageData::FrontCover || types &
> EmbeddedImageData::AllImages) {
> + imageData.insert(EmbeddedImageData::FrontCover,
> d->getFrontCover(fileUrl,mimeType));
If you follow the suggestion above, you can drop the second or'ed statement.
> embeddedimagedata.cpp:68
> + if (types & EmbeddedImageData::FrontCover || types &
> EmbeddedImageData::AllImages) {
> + imageData.insert(EmbeddedImageData::FrontCover,
> d->getFrontCover(fileUrl,mimeType));
> + }
missing space after `,`
you can also directly pass in fileMimeType.name()
> embeddedimagedata.h:45
> + AllImages = 0x01,
> + FrontCover = 0x02
> + };
I would prefer `FrontCover = 0x1`, `AllImages = FrontCover`, where AllImages is
extended later when other types are supported.
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D12320
To: astippich, mgallien, michaelh, bruns
Cc: anthonyfieroni, kde-frameworks-devel, #baloo, bruns, ashaposhnikov,
michaelh, astippich, spoorun, ngraham
