D9672: Don't emit error signal before we tried all list commands.

[Albert Astals Cid]

aacid closed this revision.
aacid added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.


  This already landed in bc4ea213b1d948b921236db29e78846c34496db7 


REPOSITORY
  R241 KIO

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

To: wcpan, dfaure
Cc: kde-frameworks-devel, aacid, ngraham, cfeck, michaelh, bruns, #frameworks

D9672: Don't emit error signal before we tried all list commands.

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  fix-ftp-list

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

To: wcpan, dfaure
Cc: ngraham, cfeck, #frameworks, michaelh

D9672: Don't emit error signal before we tried all list commands.

[Wei-Cheng Pan]

wcpan updated this revision to Diff 26460.
wcpan added a comment.


  Don't emit error signal before we tried all list commands.
  
  Use a special error code to skip error reporting.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9672?vs=24747&id=26460

BRANCH
  fix-ftp-list

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

AFFECTED FILES
  src/ioslaves/ftp/ftp.cpp

To: wcpan, dfaure
Cc: ngraham, cfeck, #frameworks, michaelh

D9672: Don't emit error signal before we tried all list commands.

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  ERR_* is KIO-wide, and isn't technically an enum.
  
  But the proper error code for no error is KJob::NoError (which is 0). Can you 
switch to that, and adjust the second comment, and kill the first comment 
altogether? (redundant with code and with 2nd comment)
  
  Thanks! (and sorry for the delay, too)

REPOSITORY
  R241 KIO

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

To: wcpan, dfaure
Cc: cfeck, #frameworks, michaelh, ngraham

D9672: Don't emit error signal before we tried all list commands.

[Christoph Feck]

cfeck edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: wcpan, dfaure
Cc: cfeck, #frameworks, michaelh, ngraham

D9672: Don't emit error signal before we tried all list commands.

[Christoph Feck]

cfeck added a comment.


  Sorry for the late response, but we currently have no maintainer for the FTP 
code in KIO.
  
  From quickly checking, the intention looks good, but if the ERR_* codes are 
part of an enum, maybe add the special value -1 with a name there, instead of 
hardcoding the -1 value? It might even fail with compilers that strictly treat 
enum's as a separate data type.

REPOSITORY
  R241 KIO

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

To: wcpan, dfaure
Cc: cfeck, #frameworks, michaelh, ngraham