bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> jobtest.cpp:471
> +
> +void JobTest::setXattr(const QString &src)
> +{
should be `dest`.
> jobtest.cpp:489
> + qDebug() << resultdest;
> + m_SkipXattr = true;
> + }
Should have a `break` or `return` here.
> arrowd wrote in jobtest.cpp:780
> To be honest, I was confused by your first comment. What was wrong with the
> first version of this code?
>
> My understanding was that we want to skip xattr stuff if either source or
> destination doesn't support it
`setXattr`, called from `checkXattrFsSupport`, unconditionally sets
`m_SkipXattr`.
You want something like
bool canRead = checkXattrFsSupport(homeDir);
bool canWrite = checkXattrFsSupport(otherHomeDir);
if (canRead && canWrite) {
and get rid of the `m_SkipXAttr` variable.
REVISION DETAIL
https://phabricator.kde.org/D17816
To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann,
funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun,
nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh
