D21760: Add KListOpenFilesJob

dfaure added a comment. The test fails on FreeBSD, in CI. https://build.kde.org/job/Frameworks/view/Platform%20-%20FreeBSDQt5.13/job/kcoreaddons/job/kf5-qt5%20FreeBSDQt5.13/118/testReport/junit/projectroot/autotests/klistopenfilesjobtest_unix/ Any plans for fixing it, or should I mar

D21760: Add KListOpenFilesJob

meven added a comment. Now we can use it : > dolphin D19989 > plasma-vault > libksysguard (KLsofWidget) > dataengines/devicenotifications/ksolidnotify REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hall

D21760: Add KListOpenFilesJob

hallas closed this revision. 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

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65856. hallas added a comment. Updated @since REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65500&id=65856 BRANCH add_list_processes_with_open_files (branched from master) REVISION DETAIL https://phab

D21760: Add KListOpenFilesJob

dfaure added a comment. The tagging has been done, you can push. REPOSITORY R244 KCoreAddons BRANCH add_list_processes_with_open_files (branched from master) REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma Cc:

D21760: Add KListOpenFilesJob

hallas added a comment. In D21760#526652 , @dfaure wrote: > This is good to go in :-) > > If you push it today it'll indeed be in 5.62, to be tagged tomorrow, otherwise we'll need to adjust the @since tag ;-) I think i prefer waiting

D21760: Add KListOpenFilesJob

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. This is good to go in :-) If you push it today it'll indeed be in 5.62, to be tagged tomorrow, otherwise we'll need to adjust the @since tag ;-) REPOSITORY R244 KCoreAddons BRANCH

D21760: Add KListOpenFilesJob

hallas added inline comments. INLINE COMMENTS > dfaure wrote in klistopenfilesjobtest_unix.cpp:34 > (minor) we never check that `new` succeeded, in Qt code. > > The reasoning is that on desktop systems, with swap enabled, before the swap > is exhausted, the user will have given up and rebooted

D21760: Add KListOpenFilesJob

hallas edited the summary of this revision. 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

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65500. hallas marked 7 inline comments as done. hallas added a comment. Review comments REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65494&id=65500 BRANCH add_list_processes_with_open_files (branched fr

D21760: Add KListOpenFilesJob

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Yep, much simpler indeed. INLINE COMMENTS > klistopenfilesjobtest_unix.cpp:34 > +auto job = new KListOpenFilesJob(path.path()); > +QVERIFY(job); > +job->exec(); (mino

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65494. hallas added a comment. Review comments, renamed the files to match the class name REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65330&id=65494 BRANCH add_list_processes_with_open_files (branched

D21760: Add KListOpenFilesJob

hallas marked an inline comment as done. hallas added a comment. In D21760#525842 , @dfaure wrote: > Yes, the filenames should match the classname, obviously :) > > I did a deeper review of the KJob usage and I have two more comments, sorry f

D21760: Add KListOpenFilesJob

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Yes, the filenames should match the classname, obviously :) I did a deeper review of the KJob usage and I have two more comments, sorry for not taking the time to do this earl

D21760: Add KListOpenFilesJob

hallas added inline comments. INLINE COMMENTS > klistopenfiles.h:23 > + > +#ifndef KLISTOPENFILES_H > +#define KLISTOPENFILES_H Should these files be renamed to klistopenfilesjob (along with tests etc.) now that is what the class is called? REPOSITORY R244 KCoreAddons REVISION DETAIL http

D21760: Add KListOpenFilesJob

hallas added a comment. In D21760#524860 , @dfaure wrote: > Given that the namespace doesn't contain anything else anymore, I would just get rid of it, and provide a single class, KListOpenFilesJob. Done :) REPOSITORY R244 KCoreAddons

D21760: Add KListOpenFilesJob

hallas updated this revision to Diff 65330. hallas added a comment. Removed KListOpenFiles namespace and renamed ListOpenFilesJob to KListOpenFilesJob REPOSITORY R244 KCoreAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21760?vs=65284&id=65330 BRANCH add_list_processes_w

D21760: Add KListOpenFilesJob

hallas retitled this revision from "Add KListOpenFiles::ListOpenFilesJob" to "Add KListOpenFilesJob". hallas edited the summary of this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D21760 To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #