Re: Review Request 114219: Do not encode QString to QByteArray and cast it back to QString. This causes problem when there are Unicode characters in ${HOME}
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114219/#review44926 --- Ship it! Yes, clearly correct. For your question about branches: commit in the stable branch and merge upwards (or ask the module maintainers to merge upwards). - David Faure On Nov. 30, 2013, 7:55 p.m., Yichao Yu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114219/ --- (Updated Nov. 30, 2013, 7:55 p.m.) Review request for kde-workspace, David Faure, Martin Gräßlin, and Hugo Pereira Da Costa. Bugs: 327919 http://bugs.kde.org/show_bug.cgi?id=327919 Repository: kde-workspace Description --- list.join already returns a QString and there is no need to encode it and cast back to QString again P.S. for a patch that applies to both KDE4 and KF5(master for kde-workspace, frameworks for kdelibs?) How should I submit review request? Should I add both in branch or submit two review request? (But often the patch cannot apply directly due to context or file path changes). Diffs - kcontrol/krdb/krdb.cpp 92d84e9 Diff: http://git.reviewboard.kde.org/r/114219/diff/ Testing --- Compiles. Fixes the problem here. Also fixes the problem for the reporter. Thanks, Yichao Yu
Re: Review Request 114201: define property X-KDE-PluginKeyword in kdelibs/kio/kcmodule.desktop
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114201/#review44927 --- Ship it! kio/kcmodule.desktop http://git.reviewboard.kde.org/r/114201/#comment32120 I see that this TODO is coming back to me :-) # The keyword to be used when loading the plugin using KPluginFactory (to support multiple KCModules in a single plugin). See KService::pluginKeyword(). - David Faure On Nov. 29, 2013, 12:35 p.m., Burkhard Lück wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114201/ --- (Updated Nov. 29, 2013, 12:35 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- This is required to make https://git.reviewboard.kde.org/r/111851/ work properly I don't know what to document here, the only hint I found in: http://websvn.kde.org/?view=revisionrevision=705672 Log Message: port to KPluginFactory Diffs - kio/kcmodule.desktop 2a978a5 Diff: http://git.reviewboard.kde.org/r/114201/diff/ Testing --- Thanks, Burkhard Lück
Re: Best practice for libraries supporting both Qt4 and Qt5
On Thursday 28 November 2013, Michael Palimaka wrote: Any thoughts? In an ideal world we would convince the distros to enable Qt namespace on Qt5, so that Qt4 and Qt5 symbols does not clash, and does not cause crashes when loaded as plugins etc. `Allan
Re: Review Request 114219: Do not encode QString to QByteArray and cast it back to QString. This causes problem when there are Unicode characters in ${HOME}
On Dec. 1, 2013, 3:47 a.m., David Faure wrote: Yes, clearly correct. For your question about branches: commit in the stable branch and merge upwards (or ask the module maintainers to merge upwards). Thank you for the review. I don't have a git account yet (will apply soon) so could you please push that for me? I would also be very nice if you can cherry-pick this patch as well as these two[1][2] to the corresponding framework/master branches. (All of them should apply straightforwardly with difference only in the context or file paths). Thank you. [1] https://git.reviewboard.kde.org/r/113939/ [2] https://git.reviewboard.kde.org/r/113985/ - Yichao --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114219/#review44926 --- On Nov. 30, 2013, 2:55 p.m., Yichao Yu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114219/ --- (Updated Nov. 30, 2013, 2:55 p.m.) Review request for kde-workspace, David Faure, Martin Gräßlin, and Hugo Pereira Da Costa. Bugs: 327919 http://bugs.kde.org/show_bug.cgi?id=327919 Repository: kde-workspace Description --- list.join already returns a QString and there is no need to encode it and cast back to QString again P.S. for a patch that applies to both KDE4 and KF5(master for kde-workspace, frameworks for kdelibs?) How should I submit review request? Should I add both in branch or submit two review request? (But often the patch cannot apply directly due to context or file path changes). Diffs - kcontrol/krdb/krdb.cpp 92d84e9 Diff: http://git.reviewboard.kde.org/r/114219/diff/ Testing --- Compiles. Fixes the problem here. Also fixes the problem for the reporter. Thanks, Yichao Yu
Re: Best practice for libraries supporting both Qt4 and Qt5
Allan Sandfeld Jensen wrote: On Thursday 28 November 2013, Michael Palimaka wrote: Any thoughts? In an ideal world we would convince the distros to enable Qt namespace on Qt5, so that Qt4 and Qt5 symbols does not clash, and does not cause crashes when loaded as plugins etc. Sounds pretty nice, wonder why that is not enabled by default then. -- Rex
Re: Review Request 112463: Port SMB kioslave to KF5/Qt5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112463/ --- (Updated Dec. 1, 2013, 9:32 p.m.) Review request for KDE Runtime and KDE Frameworks. Changes --- Updated diff since Dawit is done with his patches. I tried to be as thorough as possible with this porting. You won't see any new notices or warning coming from this patch. I at least didn't saw any in my test compiles. As for testing this. I did manage to test it and got file listings back. So that makes me think that it's ported properly and working. But i have to say that testing this is very tricky! The applications that one would normally use to verify if everything is OK (aka, dolphin) is not possible because dolphin isn't ported. Testing it in my new app Accretion is possible, but not reliable since that is in heavy development. If something doesn't show up there it doesn't mean the slave is broken :) So this patch is partly based on guess work. It looks like it's working fine. On the other hand, if it isn't i'm likely the one finding it out anyway since i'm digging around a lot in this area lately. Repository: kde-runtime Description --- This is the initial port! I added two TODO lines in the diff for parts where i'm not sure if I've ported them correctly. Also, i needed a change in FindSamba.cmake to even get the samba detection working. That reviewrequest is waiting here: https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use samba 3.x Once i know that this is actually working then i will comment some qDebug lines. Diffs (updated) - kioslave/CMakeLists.txt fc594e4 kioslave/smb/CMakeLists.txt a3a2265 kioslave/smb/kio_smb.h c2229ab kioslave/smb/kio_smb.cpp 2c2523a kioslave/smb/kio_smb_auth.cpp 4d236b4 kioslave/smb/kio_smb_browse.cpp 5253be9 kioslave/smb/kio_smb_dir.cpp ba80c86 kioslave/smb/kio_smb_file.cpp 206526a kioslave/smb/kio_smb_internal.h 4b946c1 kioslave/smb/kio_smb_internal.cpp e943844 kioslave/smb/kio_smb_mount.cpp a5a7e8e kioslave/smb/kio_smb_win.h f1dcb6f kioslave/smb/kio_smb_win.cpp 14dd797 Diff: http://git.reviewboard.kde.org/r/112463/diff/ Testing --- It compiles and gets loaded just fine. I tried testing this on an actual samba share, but i kept getting a 111 error (connection refused) from kio_smb so i'm hoping that is a local issue here. If someone else could try this out and verify that it's either working or broken. Thanks, Mark Gaiser
Re: Review Request 114219: Do not encode QString to QByteArray and cast it back to QString. This causes problem when there are Unicode characters in ${HOME}
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114219/#review44972 --- This review has been submitted with commit f10131de05ae979cdc7333f5ccfecbeb27265785 by Martin Gräßlin on behalf of Yichao Yu to branch KDE/4.11. - Commit Hook On Nov. 30, 2013, 7:55 p.m., Yichao Yu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114219/ --- (Updated Nov. 30, 2013, 7:55 p.m.) Review request for kde-workspace, David Faure, Martin Gräßlin, and Hugo Pereira Da Costa. Bugs: 327919 http://bugs.kde.org/show_bug.cgi?id=327919 Repository: kde-workspace Description --- list.join already returns a QString and there is no need to encode it and cast back to QString again P.S. for a patch that applies to both KDE4 and KF5(master for kde-workspace, frameworks for kdelibs?) How should I submit review request? Should I add both in branch or submit two review request? (But often the patch cannot apply directly due to context or file path changes). Diffs - kcontrol/krdb/krdb.cpp 92d84e9 Diff: http://git.reviewboard.kde.org/r/114219/diff/ Testing --- Compiles. Fixes the problem here. Also fixes the problem for the reporter. Thanks, Yichao Yu
Re: Review Request 114219: Do not encode QString to QByteArray and cast it back to QString. This causes problem when there are Unicode characters in ${HOME}
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114219/ --- (Updated Dec. 2, 2013, 7:27 a.m.) Status -- This change has been marked as submitted. Review request for kde-workspace, David Faure, Martin Gräßlin, and Hugo Pereira Da Costa. Bugs: 327919 http://bugs.kde.org/show_bug.cgi?id=327919 Repository: kde-workspace Description --- list.join already returns a QString and there is no need to encode it and cast back to QString again P.S. for a patch that applies to both KDE4 and KF5(master for kde-workspace, frameworks for kdelibs?) How should I submit review request? Should I add both in branch or submit two review request? (But often the patch cannot apply directly due to context or file path changes). Diffs - kcontrol/krdb/krdb.cpp 92d84e9 Diff: http://git.reviewboard.kde.org/r/114219/diff/ Testing --- Compiles. Fixes the problem here. Also fixes the problem for the reporter. Thanks, Yichao Yu
