bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.


> 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 
You want something like

  bool canRead =  checkXattrFsSupport(homeDir);
  bool canWrite =  checkXattrFsSupport(otherHomeDir);
  if (canRead && canWrite) {

and get rid of the `m_SkipXAttr` variable.


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

Reply via email to