D21760: Add KListOpenFiles::listProcessesWithOpenFiles

David Faure noreply at phabricator.kde.org
Mon Sep 2 14:33:11 BST 2019


dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Nice job. Final comments.

INLINE COMMENTS

> klistopenfilestest_unix.cpp:51
> +    QCOMPARE(processInfoAvailableSpy.at(0).at(0).value<QString>(), path.path());
> +    KProcessList::KProcessInfoList processInfoList = processInfoAvailableSpy.at(0).at(1).value<KProcessList::KProcessInfoList>();
> +    QVERIFY(!processInfoList.empty());

prepend `const` so begin/end below don't detach

> klistopenfilestest_unix.cpp:83
> +    QTemporaryDir tempDir;
> +    tempDir.remove();
> +    auto job = KListOpenFiles::listProcessesWithOpenFiles(tempDir.path());

I usually just use "/does/not/exist" as a path ;-)

This might actually be better because Windows has weird race conditions with the filesystem stuff.

> klistopenfiles.h:94
> + */
> +KCOREADDONS_EXPORT ListOpenFilesJob *listProcessesWithOpenFiles(const QString &path);
> +

KIO is designed like this for some reason, but I would just document the job constructor and let people create the job with new, outside KIO.

That's e.g. what akonadi does.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D21760

To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma
Cc: meven, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190902/907aa644/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list