Re: Review Request 123514: Make it possible to treat non-sequential QIODevice asynchronously
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123514/#review79774 --- Ship it! Well, you could still set local variables in the lambda and test them after exec, to avoid the issue that Q_ASSERT doesn't work in release mode and isn't as precise as QCOMPARE in case of failure, in debug mode. - David Faure On May 2, 2015, 12:57 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123514/ --- (Updated May 2, 2015, 12:57 a.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- So far, we used to just read whenever some data was required. This works on sequential devices because the data is already available. This is not the case when we have a sequential device, such as a socket, where data arrives when it arrives. This will also prove useful on non-sequential devices as well when we want to keep reading in case new data appears. This patch takes the AsyncDataEnabled setting on accordinly by: * only reading from the device when readyRead is available. * finishes the transfer whenever the device is closed. Diffs - src/core/transferjob.cpp 97a724e src/widgets/accessmanager.cpp b4ec811 src/core/job_p.h 7ec1a69 src/core/transferjob.h e2fd2e7 autotests/jobtest.cpp 327470a autotests/accessmanagertest.cpp 5d52553 autotests/jobtest.h 5ccd492 autotests/CMakeLists.txt 7bba3ea Diff: https://git.reviewboard.kde.org/r/123514/diff/ Testing --- Tests still pass, new test also passes. The test is using lambdas to delay write. I don't think it's available. Can I add some kind of #if HAS_LAMBDA and make the test depend on it? I don't think adding slots and make the buffer an attribute would be very nice... I can also sub-class the buffer. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123508: Shortcuts broken when user sets secondary shortcut
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123508/#review79773 --- Ship it! Ship It! - David Faure On May 1, 2015, 9:55 p.m., Lindsay Roberts wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123508/ --- (Updated May 1, 2015, 9:55 p.m.) Review request for KDE Frameworks and David Faure. Bugs: 337131, 339243, 340803, and 345411 https://bugs.kde.org/show_bug.cgi?id=337131 https://bugs.kde.org/show_bug.cgi?id=339243 https://bugs.kde.org/show_bug.cgi?id=340803 https://bugs.kde.org/show_bug.cgi?id=345411 Repository: kxmlgui Description --- When a user configures both a primary and alternate shortcut for an action, they were lost on subsequent load of the app/part .rc file. These values are persisted in the .rc file as two key sequence strings separated by ; -- the format understood by `QKeySequence::listFromString`. When these files are reparsed, the current execution flow simply calls `QObject::setProperty(shortcut, semicolonSeparatedString)`, which is `Q_PROPERTY` bound in QAction to `setShortcut(QKeySequence)` -- invoking the (single) QKeySequence constructor. The embedded semicolon and secondary shortcut seem to break this constructor. There is another specific flow in `KXMLGUIFactoryPrivate::configureAction`; one that specifically calls action-setShortcuts(QKeySequence::listFromString(attribute.value())); but it was conditionalised by: propertyType == QVariant::UserType action-property(attrName.toLatin1().constData()).userType() == qMetaTypeIdQListQKeySequence () I have not been able to track down in history when that conditional would've run true for the QAction property `shortcut`, QVariant::KeySequence was added Nov 2011, and the conditional has been there since at least the monolithic git split Dec 2013. Nor have I been able to track down any recent changes on the Qt side that would've implied the QKeySequence string constructor would've worked for multiple shortcuts in the recent past. In any case, the fix is simply to add a second conditional - matching on QVariant::KeySequence. Diffs - autotests/kxmlgui_unittest.cpp 404d304 src/kxmlguifactory.cpp aeeb242 autotests/kxmlgui_unittest.h fa2d7fb Diff: https://git.reviewboard.kde.org/r/123508/diff/ Testing --- Primarily using trunk Konsole - saving/loading/using multiple/single shortcuts. Thanks, Lindsay Roberts ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123508: Shortcuts broken when user sets secondary shortcut
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123508/ --- (Updated May 2, 2015, 9:04 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Changes --- Submitted with commit c7b1b1b89e388779356a5dcde291d6e6eac7705b by Lindsay Roberts to branch master. Bugs: 337131, 339243, 340803, and 345411 https://bugs.kde.org/show_bug.cgi?id=337131 https://bugs.kde.org/show_bug.cgi?id=339243 https://bugs.kde.org/show_bug.cgi?id=340803 https://bugs.kde.org/show_bug.cgi?id=345411 Repository: kxmlgui Description --- When a user configures both a primary and alternate shortcut for an action, they were lost on subsequent load of the app/part .rc file. These values are persisted in the .rc file as two key sequence strings separated by ; -- the format understood by `QKeySequence::listFromString`. When these files are reparsed, the current execution flow simply calls `QObject::setProperty(shortcut, semicolonSeparatedString)`, which is `Q_PROPERTY` bound in QAction to `setShortcut(QKeySequence)` -- invoking the (single) QKeySequence constructor. The embedded semicolon and secondary shortcut seem to break this constructor. There is another specific flow in `KXMLGUIFactoryPrivate::configureAction`; one that specifically calls action-setShortcuts(QKeySequence::listFromString(attribute.value())); but it was conditionalised by: propertyType == QVariant::UserType action-property(attrName.toLatin1().constData()).userType() == qMetaTypeIdQListQKeySequence () I have not been able to track down in history when that conditional would've run true for the QAction property `shortcut`, QVariant::KeySequence was added Nov 2011, and the conditional has been there since at least the monolithic git split Dec 2013. Nor have I been able to track down any recent changes on the Qt side that would've implied the QKeySequence string constructor would've worked for multiple shortcuts in the recent past. In any case, the fix is simply to add a second conditional - matching on QVariant::KeySequence. Diffs - autotests/kxmlgui_unittest.cpp 404d304 src/kxmlguifactory.cpp aeeb242 autotests/kxmlgui_unittest.h fa2d7fb Diff: https://git.reviewboard.kde.org/r/123508/diff/ Testing --- Primarily using trunk Konsole - saving/loading/using multiple/single shortcuts. Thanks, Lindsay Roberts ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123588: Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/#review79775 --- autotests/kdelibs4configmigratortest.cpp (line 85) https://git.reviewboard.kde.org/r/123588/#comment54638 You test after copy file ? It's perhaps more logical to test before no ? - Laurent Montel On mai 2, 2015, 8:17 matin, David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/ --- (Updated mai 2, 2015, 8:17 matin) Review request for KDE Frameworks and Laurent Montel. Repository: kcoreaddons Description --- Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG. Diffs - autotests/kdelibs4configmigratortest.cpp 0affd2a6bc86f8e4cad04dd662e1298d34b1e7c1 Diff: https://git.reviewboard.kde.org/r/123588/diff/ Testing --- Still passes on Linux, we'll see if the CI says it fixed it for Mac. Failure before the fix: https://build.kde.org/view/Frameworks%20kf5-qt5/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/17/testReport/junit/%28root%29/TestSuite/kdelibs4configmigratortest/ Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123589: kioexec: Fix path for writable location for kurl
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123589/#review79779 --- Ship it! Ooops! Good find, thanks. - David Faure On May 2, 2015, 8:38 a.m., Boris Egorov wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123589/ --- (Updated May 2, 2015, 8:38 a.m.) Review request for KDE Frameworks and David Faure. Bugs: 342732 https://bugs.kde.org/show_bug.cgi?id=342732 Repository: kio Description --- There was a porting issue with this code: QStandardPaths::writableLocation do not creates a directory [1], so we need QDir::mkpath. Commit ff412e0bd3ba54 tried to fix it, but it gone too far: we need to create directory only up to /krun/, strings after that is filename. So we were creating directory instead of file, confusing some apps. For example, kate [2]. 1: http://doc.qt.io/qt-5/qstandardpaths.html#writableLocation 2: https://bugs.kde.org/show_bug.cgi?id=343329 Diffs - src/kioexec/main.cpp 8fb7ef3 Diff: https://git.reviewboard.kde.org/r/123589/diff/ Testing --- New behavior was tested by removing CacheLocation, and it successfully creates needed path. Thanks, Boris Egorov ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123588: Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/#review79780 --- Ship it! Ship it - Laurent Montel On mai 2, 2015, 8:17 matin, David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/ --- (Updated mai 2, 2015, 8:17 matin) Review request for KDE Frameworks and Laurent Montel. Repository: kcoreaddons Description --- Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG. Diffs - autotests/kdelibs4configmigratortest.cpp 0affd2a6bc86f8e4cad04dd662e1298d34b1e7c1 Diff: https://git.reviewboard.kde.org/r/123588/diff/ Testing --- Still passes on Linux, we'll see if the CI says it fixed it for Mac. Failure before the fix: https://build.kde.org/view/Frameworks%20kf5-qt5/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/17/testReport/junit/%28root%29/TestSuite/kdelibs4configmigratortest/ Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123590: ftp slave: Fix conditions for QFile::rename
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123590/#review79777 --- Ship it! Indeed, bad porting from ::rename() in 4cf434b4e892. - David Faure On May 2, 2015, 8:38 a.m., Boris Egorov wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123590/ --- (Updated May 2, 2015, 8:38 a.m.) Review request for KDE Frameworks and David Faure. Bugs: 343329 https://bugs.kde.org/show_bug.cgi?id=343329 Repository: kio Description --- QFile::rename returns true on success[1], so code should treat it appropriately. Previously code shows an error when all worked fine. 1: http://doc.qt.io/qt-5/qfile.html#rename Diffs - src/ioslaves/ftp/ftp.cpp 7036f22 Diff: https://git.reviewboard.kde.org/r/123590/diff/ Testing --- Tested with kioexec and Kate. kioexec cat ftp://ftp.gnu.org/welcome.msg kate ftp://ftp.gnu.org/welcome.msg File from ftp loads and showed successfully. Thanks, Boris Egorov ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123588: Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG.
On mai 2, 2015, 8:50 matin, David Faure wrote: autotests/kdelibs4configmigratortest.cpp, line 85 https://git.reviewboard.kde.org/r/123588/diff/1/?file=365397#file365397line85 The copy that happens above is for creating the KDE4 test files, right? My QCOMPARE is to ensure that the KF5 file doesn't exist yet. I'm just reusing that foreach loop, really. The point is to then check the same after migrate() and see that the KF5 file was created. I can repeat the foreach() if you think it's more readable. Oh you're right. Ok I didn't see that it was the code which created kde4 file . So all is good indeed. - Laurent --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/#review79776 --- On mai 2, 2015, 8:17 matin, David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/ --- (Updated mai 2, 2015, 8:17 matin) Review request for KDE Frameworks and Laurent Montel. Repository: kcoreaddons Description --- Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG. Diffs - autotests/kdelibs4configmigratortest.cpp 0affd2a6bc86f8e4cad04dd662e1298d34b1e7c1 Diff: https://git.reviewboard.kde.org/r/123588/diff/ Testing --- Still passes on Linux, we'll see if the CI says it fixed it for Mac. Failure before the fix: https://build.kde.org/view/Frameworks%20kf5-qt5/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/17/testReport/junit/%28root%29/TestSuite/kdelibs4configmigratortest/ Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123589: kioexec: Fix path for writable location for kurl
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123589/ --- (Updated May 2, 2015, 11:03 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Changes --- Submitted with commit b290473167b9d0388715fffe494ee95a5c2c2851 by Boris Egorov to branch master. Bugs: 342732 https://bugs.kde.org/show_bug.cgi?id=342732 Repository: kio Description --- There was a porting issue with this code: QStandardPaths::writableLocation do not creates a directory [1], so we need QDir::mkpath. Commit ff412e0bd3ba54 tried to fix it, but it gone too far: we need to create directory only up to /krun/, strings after that is filename. So we were creating directory instead of file, confusing some apps. For example, kate [2]. 1: http://doc.qt.io/qt-5/qstandardpaths.html#writableLocation 2: https://bugs.kde.org/show_bug.cgi?id=343329 Diffs - src/kioexec/main.cpp 8fb7ef3 Diff: https://git.reviewboard.kde.org/r/123589/diff/ Testing --- New behavior was tested by removing CacheLocation, and it successfully creates needed path. Thanks, Boris Egorov ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123590: ftp slave: Fix conditions for QFile::rename
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123590/ --- (Updated May 2, 2015, 11:03 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Changes --- Submitted with commit e5c3f7eda489ee22c2f9f266884faa43f444f03c by Boris Egorov to branch master. Bugs: 343329 https://bugs.kde.org/show_bug.cgi?id=343329 Repository: kio Description --- QFile::rename returns true on success[1], so code should treat it appropriately. Previously code shows an error when all worked fine. 1: http://doc.qt.io/qt-5/qfile.html#rename Diffs - src/ioslaves/ftp/ftp.cpp 7036f22 Diff: https://git.reviewboard.kde.org/r/123590/diff/ Testing --- Tested with kioexec and Kate. kioexec cat ftp://ftp.gnu.org/welcome.msg kate ftp://ftp.gnu.org/welcome.msg File from ftp loads and showed successfully. Thanks, Boris Egorov ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
CI for kf5-qt5 is not green anymore
On my checklist before making a KF5 release: * Ensure that CI is green. What a surprise today CI is definitely not green! https://build.kde.org/view/Frameworks%20kf5-qt5/ One thing is: Mac OSX was added, so many jobs are green on linux, failing tests on mac. That's OK for now, no regression there. I started to fix KConfig unittests for mac (no need for a mac anymore, one can find out what happens by looking at CI output and make fixes), and I encourage everyone else to do the same (*). It would still be nice to see from the front page above whether a yellow job is because of Linux or Mac, so I don't have to open them all to find out what should be fixed urgently for the release. Also and more importantly: something broke with e.g. KIO on Linux: https://build.kde.org/view/Frameworks%20kf5-qt5/job/kio%20master%20kf5-qt5/39/PLATFORM=Linux,compiler=gcc/testReport/ Many of these tests fail with the following error message: couldn't create slave: Can not find io-slave for protocol 'file'. Ben, Scarlett: did something change in the setup? Is XDG_DATA_DIRS not pointing to the install dir of the framework anymore? (*) On the Mac, many tests fail with Cannot create window: no screens available. I think this is because they need a bundle, which can be done by adding GUI to the ecm_add_test call, see kxmlgui 9abace028 for an example. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 123588: Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/ --- Review request for KDE Frameworks and Laurent Montel. Repository: kcoreaddons Description --- Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG. Diffs - autotests/kdelibs4configmigratortest.cpp 0affd2a6bc86f8e4cad04dd662e1298d34b1e7c1 Diff: https://git.reviewboard.kde.org/r/123588/diff/ Testing --- Still passes on Linux, we'll see if the CI says it fixed it for Mac. Failure before the fix: https://build.kde.org/view/Frameworks%20kf5-qt5/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/17/testReport/junit/%28root%29/TestSuite/kdelibs4configmigratortest/ Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: CI for kf5-qt5 is not green anymore
On Saturday 02 May 2015 10:00:15 David Faure wrote: (*) On the Mac, many tests fail with Cannot create window: no screens available. I think this is because they need a bundle, which can be done by adding GUI to the ecm_add_test call, see kxmlgui 9abace028 for an example. Hmm, that's not enough to fix it. https://build.kde.org/view/Frameworks%20kf5-qt5/job/kxmlgui%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/23/console definitely created a bundle for e.g. kmainwindow_unittest, but still : KMainWindow_UnitTest::testDefaultName() Cannot create window: no screens available So now this looks like a CI setup issue? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123590: ftp slave: Fix conditions for QFile::rename
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123590/ --- (Updated May 2, 2015, 8:38 a.m.) Review request for KDE Frameworks and David Faure. Changes --- Add kdeframeworks as review group Bugs: 343329 https://bugs.kde.org/show_bug.cgi?id=343329 Repository: kio Description --- QFile::rename returns true on success[1], so code should treat it appropriately. Previously code shows an error when all worked fine. 1: http://doc.qt.io/qt-5/qfile.html#rename Diffs - src/ioslaves/ftp/ftp.cpp 7036f22 Diff: https://git.reviewboard.kde.org/r/123590/diff/ Testing --- Tested with kioexec and Kate. kioexec cat ftp://ftp.gnu.org/welcome.msg kate ftp://ftp.gnu.org/welcome.msg File from ftp loads and showed successfully. Thanks, Boris Egorov ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123589: kioexec: Fix path for writable location for kurl
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123589/ --- (Updated May 2, 2015, 8:38 a.m.) Review request for KDE Frameworks and David Faure. Changes --- Add kdeframeworks as review group Bugs: 342732 https://bugs.kde.org/show_bug.cgi?id=342732 Repository: kio Description --- There was a porting issue with this code: QStandardPaths::writableLocation do not creates a directory [1], so we need QDir::mkpath. Commit ff412e0bd3ba54 tried to fix it, but it gone too far: we need to create directory only up to /krun/, strings after that is filename. So we were creating directory instead of file, confusing some apps. For example, kate [2]. 1: http://doc.qt.io/qt-5/qstandardpaths.html#writableLocation 2: https://bugs.kde.org/show_bug.cgi?id=343329 Diffs - src/kioexec/main.cpp 8fb7ef3 Diff: https://git.reviewboard.kde.org/r/123589/diff/ Testing --- New behavior was tested by removing CacheLocation, and it successfully creates needed path. Thanks, Boris Egorov ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123588: Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/#review79776 --- autotests/kdelibs4configmigratortest.cpp (line 85) https://git.reviewboard.kde.org/r/123588/#comment54639 The copy that happens above is for creating the KDE4 test files, right? My QCOMPARE is to ensure that the KF5 file doesn't exist yet. I'm just reusing that foreach loop, really. The point is to then check the same after migrate() and see that the KF5 file was created. I can repeat the foreach() if you think it's more readable. - David Faure On May 2, 2015, 8:17 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123588/ --- (Updated May 2, 2015, 8:17 a.m.) Review request for KDE Frameworks and Laurent Montel. Repository: kcoreaddons Description --- Port kdelibs4configmigratortest to QStandardPaths, so it works on other platforms than XDG. Diffs - autotests/kdelibs4configmigratortest.cpp 0affd2a6bc86f8e4cad04dd662e1298d34b1e7c1 Diff: https://git.reviewboard.kde.org/r/123588/diff/ Testing --- Still passes on Linux, we'll see if the CI says it fixed it for Mac. Failure before the fix: https://build.kde.org/view/Frameworks%20kf5-qt5/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/17/testReport/junit/%28root%29/TestSuite/kdelibs4configmigratortest/ Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: CI for kf5-qt5 is not green anymore
On Saturday, May 02, 2015 10:00:15 AM David Faure wrote: On my checklist before making a KF5 release: * Ensure that CI is green. What a surprise today CI is definitely not green! https://build.kde.org/view/Frameworks%20kf5-qt5/ One thing is: Mac OSX was added, so many jobs are green on linux, failing tests on mac. That's OK for now, no regression there. I started to fix KConfig unittests for mac (no need for a mac anymore, one can find out what happens by looking at CI output and make fixes), and I encourage everyone else to do the same (*). It would still be nice to see from the front page above whether a yellow job is because of Linux or Mac, so I don't have to open them all to find out what should be fixed urgently for the release. Due to the jobs now being Matrix to accommodate multi platforms jenkins insists on only showing the Parent. I have tried many variations to show children (aka Linux views) without success. I may have to dig into jenkins code to see what I can come up with, but as such it is not a simple fix and is added to my to-do list. There are higher priority items though, so I do not have a timeline for this. Also and more importantly: something broke with e.g. KIO on Linux: https://build.kde.org/view/Frameworks%20kf5-qt5/job/kio%20master%20kf5-qt5/3 9/PLATFORM=Linux,compiler=gcc/testReport/ Many of these tests fail with the following error message: couldn't create slave: Can not find io-slave for protocol 'file'. Ben, Scarlett: did something change in the setup? Is XDG_DATA_DIRS not pointing to the install dir of the framework anymore? This will be fixed when the builders free up so I can restart jenkins. XDG* is was not set. So things should improve after restart. (*) On the Mac, many tests fail with Cannot create window: no screens available. I think this is because they need a bundle, which can be done by adding GUI to the ecm_add_test call, see kxmlgui 9abace028 for an example. This will require more time to research. Thanks! Scarlett signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123224: KIO::suggestName suggests wrong name for some filenames
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123224/ --- (Updated May 2, 2015, 4:46 p.m.) Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Arjun AK. Changes --- fixed issues Bugs: 341773 https://bugs.kde.org/show_bug.cgi?id=341773 Repository: kio Description --- For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong name(something like filename-1 2.6.tar.gz). Expected name: filename-1.6 (1).tar.gz Diffs (updated) - autotests/globaltest.cpp ff8725d src/core/global.cpp f18ac10 Diff: https://git.reviewboard.kde.org/r/123224/diff/ Testing --- Works fine! Thanks, Ashish Bansal ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 123595: Fix KUser test for Mac.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123595/ --- Review request for KDE Frameworks and Marko Käning. Repository: kcoreaddons Description --- According to CI [1], an invalid user belongs to nogroup on Mac. Not sure if this is true on all OSX installations though? https://build.kde.org/view/Frameworks%20kf5-qt5/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/19/testReport/%28root%29/TestSuite/kusertest/ Diffs - autotests/kusertest.cpp d17a2d3e97d5056524281eb18766377e48a0da35 Diff: https://git.reviewboard.kde.org/r/123595/diff/ Testing --- Still passes on Linux; should fix the CI for mac, AFAICS. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: CI for kf5-qt5 is not green anymore
On Saturday 02 May 2015 06:56:06 Scarlett Clark wrote: This will be fixed when the builders free up so I can restart jenkins. XDG* is was not set. So things should improve after restart. Thanks. Did a restart happen by now? I tried to kick a new kparts build to see if it worked better, but I don't have a link for rebuild now anymore. Did I lose that permission somehow, or is the functionality gone? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123224: KIO::suggestName suggests wrong name for some filenames
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123224/#review79787 --- Ship it! Ship It! - David Faure On May 2, 2015, 4:46 p.m., Ashish Bansal wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123224/ --- (Updated May 2, 2015, 4:46 p.m.) Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Arjun AK. Bugs: 341773 https://bugs.kde.org/show_bug.cgi?id=341773 Repository: kio Description --- For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong name(something like filename-1 2.6.tar.gz). Expected name: filename-1.6 (1).tar.gz Diffs - autotests/globaltest.cpp ff8725d src/core/global.cpp f18ac10 Diff: https://git.reviewboard.kde.org/r/123224/diff/ Testing --- Works fine! Thanks, Ashish Bansal ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123224: KIO::suggestName suggests wrong name for some filenames
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123224/ --- (Updated May 2, 2015, 7:26 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Plasma, Aleix Pol Gonzalez, and Arjun AK. Changes --- Submitted with commit c806f88e4ea65330719fa1721cdf15ea5cbddb5a by Ashish Bansal to branch master. Bugs: 341773 https://bugs.kde.org/show_bug.cgi?id=341773 Repository: kio Description --- For filenames like filename-1.6.tar.gz, KIO::suggestName suggests wrong name(something like filename-1 2.6.tar.gz). Expected name: filename-1.6 (1).tar.gz Diffs - autotests/globaltest.cpp ff8725d src/core/global.cpp f18ac10 Diff: https://git.reviewboard.kde.org/r/123224/diff/ Testing --- Works fine! Thanks, Ashish Bansal ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: CI for kf5-qt5 is not green anymore
On Saturday 02 May 2015 10:00:15 David Faure wrote: Ben, Scarlett: did something change in the setup? Is XDG_DATA_DIRS not pointing to the install dir of the framework anymore? Similarly, shared-mime-info (found via XDG_DATA_DIRS on Linux) isn't found anymore, breaking at least kparts and kiconthemes unittests on Linux. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: CI for kf5-qt5 is not green anymore
On Saturday, May 02, 2015 08:36:02 PM David Faure wrote: On Saturday 02 May 2015 06:56:06 Scarlett Clark wrote: This will be fixed when the builders free up so I can restart jenkins. XDG* is was not set. So things should improve after restart. Thanks. Did a restart happen by now? I tried to kick a new kparts build to see if it worked better, but I don't have a link for rebuild now anymore. Did I lose that permission somehow, or is the functionality gone? Unfortunately even with XDG set it is still not functioning as expected. I need to investigate further. Permissions will return when the dust settles. Scarlett signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Opinions on KIO Slave side sorting? Possible GSoC project?
On Thursday 08 January 2015 10:45:13 Mark Gaiser wrote: (wow, time flies) The issue i see here is different processes. KIO::listDir is a process, KDirListerCache lives in the client process. I guess you didn't mean this litterally, because in that case it's wrong, both happen in the client process. You want to do sorting in both processes. 1. When the client initially asks a listing you want to do the sorting as quick as possible to send back as little data as possible. So sorting on the SlaveBase side. As long as not all data is known in the KDirListerCache sorting would have to be done on the SlaveBase side. That would be the case to change sort order _while_ data is dripping in. 2. When all data is fetched you don't want to go over a socket to request a different sorting method so then you would need to have the same sorting operations on the KIO client side with data fed from KDirListerCache. Obviously more complex than the current solution, but OK, let's see where that leads us. I do think that KIO would need a new class for this that can handle a set of predefined sorting and filtering operations if this entire slave side sorting is to be considered for KIO. Just to be clear, when i said slave side sorting i meant sorting in - for instance - SlaveBase. Not in the actual slave plugins. Where exactly, i don't know, but certainly before data is being send back to the application that requested a KIO::listDir. Sorry if i confused you here. Well, this requires the actual kioslave to first give all items to SlaveBase, doesn't it? Otherwise it can't sort them. So instead of incremental listing, this would wait until everything is available and then send everything; not sure it would appear faster to the user. (this depends on whether the time is spent doing the actually listing and creating of UDSEntries, or if more time is spent sending the stuff over the socket --- but you optimized that :-) The advantage i see with doing this is allowing data to be visible for the user as soon as data is available which will give the user a smoother and faster experience. Interesting you should say that, I think the suggested approach actually goes into the opposite direction from this goal. You need for all udsentries to exist before sorting, so it might take longer for the first items to appear, compared to the current solution, where a first set of items get sent over as soon as they are available, and then the other batch comes in and the two get sorted together. Which can make items move down, so I'm not saying it's ideal, but it technically does reach the above goal better :) It allows for a mechanism of show me the first 100 items of this massive folder in this particular sort order. Something that isn't fully possible right now since the client would have to wait till all data has arrived over the socket, then sort it, then present it. All data would still have to be fetched on the slave side, but only X number of entries would have to be send to the client allowing it to immediately present the data. The rest of the data can then be fetched on a need to know basis. Eg. when scrolling down for the next batch of 100 items. By that time the slave would probably (depending on the folder size and sort order) be done fetching all items and is ready to immediately send those pre sorted batches to the client. So the very first step is to find out how long it takes to create udsentries in the slave, and how long it takes to transfer them over the socket. If the first one is much bigger than the second one, then this idea would delay quite a lot the time until when the first items appear... An added advantage is that the client could then just use KIO as streaming API. Just like a youtube api or whatever streaming api. The client would just have to implement canFetchMode and fetchMode if the Qt classes are used. It should prevent the client from needing to implement complicated threading and UI tricks to keep the UI smooth. fetchMore is pull (get me more now), while entries get listed asynchronously and pushed via a signal, so I don't see how that would work, what would fetchMore really do? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [knewstuff] /: registerServicesByCategory: add screenshot-take
Hi Gregor, does the unittest pass for you? https://build.kde.org/view/Frameworks%20kf5-qt5/job/knewstuff%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/lastBuild/testReport/%28root%29/TestSuite/kmoretoolstest/ I get the same failures on my system. In fact, is tests/kmoretoolstest supposed to be a unittest ? It uses qtestlib but then it shows a modal dialog (due to the call to menuBuilder-showConfigDialog(description) at line 482) which prevents the unittest from going further. This should be fixed to not show a modal dialog, or to have a timer triggering the closing of the dialog. I wanted to release KF 5.10 today, but releasing something with broken unittests doesn't make me very confident. Any chance all this can be stabilized by tomorrow? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
