D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> deletejob.cpp:67
> +Q_SIGNALS:
> +    void rmfileResult(bool succeeded, const QUrl& url, const bool isLink);
> +    void rmddirResult(bool succeeded, const QUrl& url);

"const bool" doesn't do anything in a signal, I suggest removing the const.

> deletejob.cpp:76
> +     */
> +    void rmfile(const QUrl& url, const bool isLink){
> +        emit rmfileResult(QFile::remove(url.toLocalFile()), url, isLink);

(here it technically does something (guarantees that it's not modified in the implementation), but that's quite unusual in Qt/KDE code; your choice)

> deletejob.cpp:412
> +        m_processedDirs++;
> +        dirs.removeFirst();
> +        deleteNextDir();

removeLast here too?

> deletejob.cpp:445
> +        // If local dir, try to rmdir it directly
> +        if ((*it).isLocalFile()) {
> +            // delete it on separate worker thread

Use m_currentURL here, or use (*it) in the Q_ARG. It's just inconsistent right now, even though of course it's all technically the same.

(I think the code wasn't using m_currentURL because that's just something used for reporting, added after the fact, and which could technically be changed again one day -- although that's unlikely, I guess).
So I don't feel strongly about which one to use, but it should be consistent.

REPOSITORY
  R241 KIO

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

To: meven, dfaure
Cc: 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/20191027/db67215f/attachment-0001.html>