D23579: port ftp slave to new error reporting system
sitter retitled this revision from "WIP: port ftp slave to new error reporting system" to "port ftp slave to new error reporting system". sitter edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23579 To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23579: port ftp slave to new error reporting system
sitter updated this revision to Diff 67697. sitter added a comment. move away from helper functions to redirect to q-> and instaed call via q-> directly REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23579?vs=67695&id=67697 BRANCH ftp REVISION DETAIL https://phabricator.kde.org/D23579 AFFECTED FILES autotests/CMakeLists.txt autotests/ftp/testCopy1 autotests/ftp/testCopy2 autotests/ftp/testOverwriteCopy1 autotests/ftp/testOverwriteCopy2 autotests/ftpd autotests/ftptest.cpp cmake/FindGem.cmake cmake/FindGem.cmake.in cmake/FindRubyExe.cmake src/ioslaves/ftp/ProxyTesting.md src/ioslaves/ftp/ftp.cpp src/ioslaves/ftp/ftp.h To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23579: port ftp slave to new error reporting system
sitter edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23579 To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23579: port ftp slave to new error reporting system
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Excellent work! INLINE COMMENTS > ftp.cpp:2635 > + > +#pragma message "was this useful? I have no clue where or how keepalive > would work with ftp" > +//authenticator->setOption(QStringLiteral("keepalive"), > info.keepPassword); yeah, looks like wishful thinking remove? > sitter wrote in ftp.h:70 > It's a good idea. Would that work though? > > Currently the results rely on the implicit move operator when collecting the > returned result > > result = ftpGet() > > if the members are const we couldn't move/copy like that anymore. > > If we consider the mutability a problem I think I'd just make the members > private and give them getters. I don't mind much either way. Copying bool+int+QString seems rather cheap, we lived with that until C++11 move semantics ;-) Private members and inline getters sounds good to me. But yeah, no big deal, we can also trust the programmer :-) REPOSITORY R241 KIO BRANCH ftp REVISION DETAIL https://phabricator.kde.org/D23579 To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23579: port ftp slave to new error reporting system
This revision was automatically updated to reflect the committed changes. sitter marked 3 inline comments as done. Closed by commit R241:c86eedf0f92d: port ftp slave to new error reporting system (authored by sitter). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D23579?vs=67697&id=67883#toc REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23579?vs=67697&id=67883 REVISION DETAIL https://phabricator.kde.org/D23579 AFFECTED FILES autotests/CMakeLists.txt autotests/ftp/testCopy1 autotests/ftp/testCopy2 autotests/ftp/testOverwriteCopy1 autotests/ftp/testOverwriteCopy2 autotests/ftpd autotests/ftptest.cpp cmake/FindGem.cmake cmake/FindGem.cmake.in cmake/FindRubyExe.cmake src/ioslaves/ftp/ProxyTesting.md src/ioslaves/ftp/ftp.cpp src/ioslaves/ftp/ftp.h To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23579: port ftp slave to new error reporting system
dfaure added a comment. @sitter The unittest just failed on CI, the test can't connect to the local FTP server, can you look into it? https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.13/211/testReport/junit/projectroot/autotests/kiocore_ftptest/ REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23579 To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23579: port ftp slave to new error reporting system
sitter added a comment. I'll see about adding some debug output for the daemon state. Hard to say why it's flakey with the info at hand. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23579 To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23579: port ftp slave to new error reporting system
dfaure added a comment. Wait, nevermind, it was a regression in the mapConfig() related code, fixed meanwhile. KIO is green again on CI. Sorry for the false alert. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23579 To: sitter, dfaure Cc: anthonyfieroni, dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns