mgallien added a comment.
Thanks for your work. Please find a few remarks inline. INLINE COMMENTS > embeddedimagedata.h:25 > +#include "kfilemetadata_export.h" > +#include <QMimeDatabase> > + You can do a forward declaration of QMimeDatabase because it is only referred through a reference. > embeddedimagedata.h:40 > + class Private; > + Private *d; > + You should be using a std::unique_ptr instead of a raw pointer. You also should take care of either forbidding copy (operator= and copy constructor) or implement them to perform a deep copy including duplicating the private object. Currently, it will be implicitly shared between instances. In many classes in KDE repository you do not need to do it because QObject inheritance gives you exactly that. > embeddedimagedata.h:42 > + > + QByteArray getFrontCover(const QString &fileUrl, const QString > &mimeType) const; > + The private method can go to the private class. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D12320 To: astippich, mgallien, michaelh Cc: bruns, #frameworks, ashaposhnikov, michaelh, astippich, spoorun
