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
