bruns added inline comments. INLINE COMMENTS
> databasesanitizer.cpp:187 > + > + bool isIgnored(const QStorageInfo& storageInfo, const > DatabaseSanitizer::ItemAccessFilters accessFilter) > + { Are you planning to use this one anywhere else? Otherwise, inside `deviceFilters(...)`: auto isIgnored = [&accessFilter](const QStorageInfo& storageInfo) { if (storageInfo.isValid() && ...) { ... } } > databasesanitizer.cpp:189 > + { > + if (storageInfo.isValid() && (accessFilter & > DatabaseSanitizer::IgnoreMounted)) { > + return true; `bool mounted = storageInfo.isValid()` > databasesanitizer.cpp:191 > + return true; > + } else if (!storageInfo.isValid() && (accessFilter & > DatabaseSanitizer::IgnoreUnmounted)) { > + return true; This `else` is ok, although not necessary, it emphasizes symmetry (mounted, not mounted) > databasesanitizer.cpp:193 > + return true; > + } else if (storageInfo.fileSystemType() == QLatin1String("tmpfs")) { > + // Due to the volatility of device ids, an id known by baloo may Remove the `else`, and add a newline here ... } if (storageInfo.fileSystemType() == ...) { ... return true; } return false; > databasesanitizer.cpp:210 > + void commit() { > + m_transaction->abort(); > + } ->commit() > databasesanitizer.cpp:373 > + err << i18nc("numbers", "Removed: %1, Total: %2, Ignored: %3", > + summary.total - summary.ignored, > + summary.total, Nitpick - one more space (align " and __s__ummary) > michaelh wrote in databasesanitizer.cpp:344 > I'm assuming you meant `DatabaseSanitizerImpl::removeDocument(info.id)`. If > you meant `DatabaseSanitizer::removeDocument`, please explain what the > advantage is. Right. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks, bruns Cc: bruns, cfeck, smithjd, ashaposhnikov, michaelh, astippich, spoorun, ngraham