dfaure added inline comments.
INLINE COMMENTS
> klistopenfiles.cpp:29
> +
> +class ListOpenFilesJobPrivate : public QObject
> +{
I wonder if this actually needs to be a QObject, given that you use
connect-to-pointer-to-member-function?
> hallas wrote in klistopenfiles.cpp:44
> Should I also set an error string here? And what about defining custom error
> codes, is that how we usually do?
KIO::ERR_DOES_NOT_EXIST would fit well here, but that's in kio...
I suggest adding enum { ERR_DOES_NOT_EXIST = KJob::UserDefinedError + 11 }
in the header file for this job and using that here.
And yes, you should do something like
setErrorText(i18n("Directory does not exist: %1", path));
> klistopenfiles.cpp:53
> + {
> + job->setError(KJob::UserDefinedError);
> + job->emitResult();
+setErrorText(<something dependent on ProcessError>)
> klistopenfiles.h:41
> +public:
> + explicit ListOpenFilesJob(QDir path);
> + ~ListOpenFilesJob() override;
const QDir & --- or actually, we use const QString & everywhere else for paths.
> klistopenfiles.h:51
> + * @param processInfoList The list of processes with open files in path
> + * @since 5.62
> + */
The @since should go into the (missing) class documentation, given that the
whole class is new.
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
