dfaure added inline comments.
INLINE COMMENTS
> copyjob.cpp:430
> + if (!m_privilegeExecutionEnabled && !isWritable) {
> + const QUrl dest = m_asMethod ?
> m_dest.adjusted(QUrl::RemoveFilename) : m_dest;
> + q->setError(ERR_WRITE_ACCESS_DENIED);
This is the same as "actualDest" too, so its definition could be moved further
up and shared with this too.
(Not that the name is perfect --- when copying ~/file.txt to ~/file2.txt, the
actual destination is ~/file2.txt.
Or copying ~/dir1 as ~/dir2, then the actual destination is ~/dir2. So even a
name like destDir isn't perfect...)
existingDest maybe. ~ exists. ~/dir2 not yet.
> copyjob.cpp:448
> const QString fileName = m_dest.fileName();
> + // The statJob that returned "entry" has taken into account
> m_asMethod
> m_dest = QUrl::fromLocalFile(sLocalPath);
It has, but "taking into account" is ambiguous. I first thought it meant "we
stat'ed the final dest", while in fact it means we stat'ed the parent existing
dest. Maybe no comment is better than an ambiguous comment :-)
> ahmadsamir wrote in copyjob.cpp:479
> s/below/below, because return is called right after it, so that
> statCurrentSrc is called from the lambda/
>
> does that sound better?
Yes. But comments should add information, not just describe what the code
already says :-)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D29485
To: ahmadsamir, #frameworks, dfaure, meven, sitter
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns
