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
