dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
I know this isn't your code and you're just moving it to solve a dependency
issue. No problem, go ahead.
But while at it I took at it, I look at the code and found a few issues, feel
free to fix them ;)
Unittests would be good, too .... OK ok I'm pushing it :)
INLINE COMMENTS
> remoteimpl.cpp:42
> + if (!dir.exists()) {
> + dir.cdUp();
> + dir.mkdir(QStringLiteral("remoteview"));
This should probably be QDir().mkpath(path) instead of the whole if exists +
cdUp + mkdir dance, which would fail if the parent doesn't exist either (ok,
unlikely, but anyway, mkpath is shorter and safer)
> remoteimpl.cpp:96
> +
> + QStringList filenames = dir.entryList(QDir::Files | QDir::Readable);
> +
Wow this is convoluted, a simple QFile::exists(*dirpath + '/' + filename)
should be enough rather than a full directory listing.
> remoteimpl.cpp:155
> + if (service && service->isValid()) {
> +
> url.setPath(QStandardPaths::locate(QStandardPaths::ApplicationsLocation,
> +
> QStringLiteral("%1.desktop").arg(WIZARD_SERVICE)));
wrong port to QUrl, should be url = QUrl::fromLocalFile(...)
REPOSITORY
R241 KIO
BRANCH
import-remote
REVISION DETAIL
https://phabricator.kde.org/D4690
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: elvisangelaccio, dfaure, davidedmundson
Cc: #plasma, #frameworks
