graesslin added a comment.
There are a few unrelated changes, but otherwise looks good. INLINE COMMENTS > package.cpp:360 > //Nested loop, but in the medium case resolves to just one iteration > - //qDebug() << "prefixes:" << prefixes.count() << prefixes; > +// qDebug() << "prefixes:" << d->contentsPrefixPaths.count() << > d->contentsPrefixPaths; > foreach (const QString &contentsPrefix, d->contentsPrefixPaths) { ? > package.cpp:594 > + if (!it.value().endsWith(QLatin1Char('/'))) { > + it.setValue(it.value() % QLatin1Char('/')); > } Just curious: what does the modulo operator on a string? > package.cpp:946 > > - if (isDir && QFile::exists(path + "/metadata.json")) { > - metadata = new KPluginMetaData(path + "/metadata.json"); > - } else if (isDir && QFile::exists(path + "/metadata.desktop")) { > - auto md = KPluginMetaData::fromDesktopFile(path + > "/metadata.desktop", > {QStringLiteral(":/kservicetypes5/kpackage-generic.desktop")}); > + delete metadata; > + if (isDir && QFile::exists(path + QStringLiteral("/metadata.json"))) { This looks like a unrelated move of the line. > kpackagetool.cpp:437 > // This can happen in the case an application wanting to support > kpackage based extensions includes in the same project both the > packagestructure plugin and the packages themselves. In that case at build > time the packagestructure plugin wouldn't be installed yet > + > if (QFile::exists(pluginName + QStringLiteral("/metadata.desktop"))) { unrelated? REPOSITORY R290 KPackage REVISION DETAIL https://phabricator.kde.org/D9104 To: apol, #frameworks, #plasma, davidedmundson Cc: graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart