dfaure added a comment.

  Phabricator usage: you can't upload one patch for two git repos into a single 
change request. This makes the "repo" field wrong, and it makes it impossible 
to get context. Use `arc diff` and create two review requests (unfortunately).
  
  Commit log: something like "fix crash during file copy after message box 
warning" would already be better than either just a bug number (v1) or a very 
generic sentence (v2).
  
  The bug looks like a nested event loop issue, which is always nasty. I don't 
have time right now for full debugging, but if you could paste a backtrace of 
where a messagebox is shown that would be helpful. I suppose it all comes from 
the slave calling warning? Warnings have never been really handled well... 
Maybe we should collect warnings and show them at the end of the job? This 
might help with these issues...
  
  KCompositeJob patch: that one looks ok, it reads like basic input validation, 
even though I would have never thought this could happen (again, nested event 
loops is most certainly the reason...)
  
  KIO patch: If there are still any subjobs running, we should kill them rather 
than patiently wait for them to finish (it's pointless, if the whole job is 
being aborted). Which subjobs are still there, in fact? This code already kills 
the put job if the get job has an error, and vice-versa. Are there more jobs 
that should be killed in that code?

REPOSITORY
  R241 KIO

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

To: jtamate, #frameworks, dfaure
Cc: ltoscano

Reply via email to