D16554: move supported mimetypes to static string list
This revision was automatically updated to reflect the committed changes. Closed by commit R286:0375eee8a128: move supported mimetypes to static string list (authored by astippich). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16554?vs=44661&id=44825 REVISION DETAIL https://phabricator.kde.org/D16554 AFFECTED FILES src/extractors/epubextractor.cpp src/extractors/exiv2extractor.cpp src/extractors/ffmpegextractor.cpp src/extractors/odfextractor.cpp src/extractors/office2007extractor.cpp src/extractors/plaintextextractor.cpp src/extractors/poextractor.cpp src/extractors/popplerextractor.cpp src/extractors/taglibextractor.cpp src/writers/taglibwriter.cpp To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
bruns accepted this revision.
bruns added inline comments.
This revision is now accepted and ready to land.
INLINE COMMENTS
> taglibextractor.cpp:71
> +QStringLiteral("audio/x-vorbis+ogg"),
> +QStringLiteral("audio/wav"),
> +QStringLiteral("audio/x-wavpack"),
w < x
REPOSITORY
R286 KFileMetaData
BRANCH
mimetype_lists
REVISION DETAIL
https://phabricator.kde.org/D16554
To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
astippich updated this revision to Diff 44661. astippich added a comment. - use local static variable REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16554?vs=44613&id=44661 BRANCH mimetype_lists REVISION DETAIL https://phabricator.kde.org/D16554 AFFECTED FILES src/extractors/epubextractor.cpp src/extractors/exiv2extractor.cpp src/extractors/ffmpegextractor.cpp src/extractors/odfextractor.cpp src/extractors/office2007extractor.cpp src/extractors/plaintextextractor.cpp src/extractors/poextractor.cpp src/extractors/popplerextractor.cpp src/extractors/taglibextractor.cpp src/writers/taglibwriter.cpp To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
bruns added inline comments. INLINE COMMENTS > astippich wrote in epubextractor.cpp:44 > All the stringlists are currently implemented as a private static member of > the class... would you like to change that to a local static variable? I consider this cleaner, yes. We don't expose static helper functions, either. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16554 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
astippich added inline comments. INLINE COMMENTS > bruns wrote in epubextractor.cpp:44 > The idea behind using anonymous namespaces is to neither "pollute" the class > namespace nor the global namespace. Remove the "EPubExtractor::" qualifier, > see `fetchMetadata`. All the stringlists are currently implemented as a private static member of the class... would you like to change that to a local static variable? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16554 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
bruns added inline comments.
INLINE COMMENTS
> epubextractor.cpp:44
> {
> +const QStringList EPubExtractor::mMimetypes = {
> +QStringLiteral("application/epub+zip"),
The idea behind using anonymous namespaces is to neither "pollute" the class
namespace nor the global namespace. Remove the "EPubExtractor::" qualifier, see
`fetchMetadata`.
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D16554
To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
astippich updated this revision to Diff 44613. astippich added a comment. - adjust taglibwriter REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16554?vs=44612&id=44613 BRANCH mimetype_lists REVISION DETAIL https://phabricator.kde.org/D16554 AFFECTED FILES src/extractors/epubextractor.cpp src/extractors/epubextractor.h src/extractors/exiv2extractor.cpp src/extractors/exiv2extractor.h src/extractors/ffmpegextractor.cpp src/extractors/ffmpegextractor.h src/extractors/odfextractor.cpp src/extractors/odfextractor.h src/extractors/office2007extractor.cpp src/extractors/office2007extractor.h src/extractors/plaintextextractor.cpp src/extractors/plaintextextractor.h src/extractors/poextractor.cpp src/extractors/poextractor.h src/extractors/popplerextractor.cpp src/extractors/popplerextractor.h src/extractors/taglibextractor.cpp src/extractors/taglibextractor.h src/writers/taglibwriter.cpp src/writers/taglibwriter.h To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
astippich updated this revision to Diff 44612. astippich added a comment. - coding style fixes - use anonymous namespace REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16554?vs=44572&id=44612 BRANCH mimetype_lists REVISION DETAIL https://phabricator.kde.org/D16554 AFFECTED FILES src/extractors/epubextractor.cpp src/extractors/epubextractor.h src/extractors/exiv2extractor.cpp src/extractors/exiv2extractor.h src/extractors/ffmpegextractor.cpp src/extractors/ffmpegextractor.h src/extractors/odfextractor.cpp src/extractors/odfextractor.h src/extractors/office2007extractor.cpp src/extractors/office2007extractor.h src/extractors/plaintextextractor.cpp src/extractors/plaintextextractor.h src/extractors/poextractor.cpp src/extractors/poextractor.h src/extractors/popplerextractor.cpp src/extractors/popplerextractor.h src/extractors/taglibextractor.cpp src/extractors/taglibextractor.h src/writers/taglibwriter.cpp src/writers/taglibwriter.h To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
bruns added inline comments.
INLINE COMMENTS
> epubextractor.cpp:38
> +const QStringList EPubExtractor::mMimetypes =
> {
> +QStringLiteral("application/epub+zip"),
Opening braces should go on the same line for everything but functions ...
> epubextractor.cpp:48
> namespace
> {
> QString fetchMetadata(struct epub* e, const epub_metadata& type)
you can put the static mimetypes variable here in the anonymous namespace
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D16554
To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
D16554: move supported mimetypes to static string list
astippich created this revision. astippich added a reviewer: bruns. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY Do not create the list of supported mime types on demand and use a static stringlist instead. Order alphabetically where required REPOSITORY R286 KFileMetaData BRANCH mimetype_lists REVISION DETAIL https://phabricator.kde.org/D16554 AFFECTED FILES src/extractors/epubextractor.cpp src/extractors/epubextractor.h src/extractors/exiv2extractor.cpp src/extractors/exiv2extractor.h src/extractors/ffmpegextractor.cpp src/extractors/ffmpegextractor.h src/extractors/odfextractor.cpp src/extractors/odfextractor.h src/extractors/office2007extractor.cpp src/extractors/office2007extractor.h src/extractors/plaintextextractor.cpp src/extractors/plaintextextractor.h src/extractors/poextractor.cpp src/extractors/poextractor.h src/extractors/popplerextractor.cpp src/extractors/popplerextractor.h src/extractors/taglibextractor.cpp src/extractors/taglibextractor.h src/writers/taglibwriter.cpp src/writers/taglibwriter.h To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
