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
