D21760: Add KListOpenFiles::listProcessesWithOpenFiles

David Faure noreply at phabricator.kde.org
Thu Aug 29 08:54:28 BST 2019


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190829/57942649/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list