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
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. @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
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=67883#toc REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23579?vs=67697=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 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
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=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 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
