Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/#review52717 --- Ship it! Ship It! - David Faure On March 10, 2014, 5:46 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 10, 2014, 5:46 p.m.) Review request for Documentation, KDE Frameworks and David Faure. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: src/kcompletionbox.h, line 228 https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228 I wouldn't leave the implementation here. Move it to the .cpp file, this way it can be changed in the future, if it's required for some reason. Also there's a typo in the method name. Alex Merry inlined deprecated methods in https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way to go... On March 11, 2014, 11:17 p.m., David Gil Oliva wrote: Have you looked through the uses of the un-slotted methods? (lxr.kde.org). Maybe there's a reason for that... :/ Maybe I'm totally wrong, but I can't imagine any way that a getter can be useful as a slot. connect (widget, SLOT(valueChanged(), completionBox, SIGNAL(isTabHandling()); Does it make sense?¿?:-/ - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116756: Remove Class Members link. It's broken and not useful.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116756/ --- Review request for KDE Frameworks and Aurélien Gâteau. Repository: kapidox Description --- Remove Class Members link. It's broken and not useful. Diffs - src/kapidox/__init__.py 32fa91b Diff: https://git.reviewboard.kde.org/r/116756/diff/ Testing --- Generated doc: no Class Members link anymore. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116699: No longer use pid_t or Q_PID
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116699/#review52731 --- there is QProcess::processId() since 5.3 (replacing KProcess::pid()). it simply returns a somewhat wasteful qint64. you may want to follow the same practice. windows does of course not have a native getpid() function, but it is easy enough to replicate. - Oswald Buddenhagen On March 12, 2014, 7:36 a.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116699/ --- (Updated March 12, 2014, 7:36 a.m.) Review request for KDE Frameworks, David Faure and Oswald Buddenhagen. Repository: kio Description --- No longer use pid_t or Q_PID Instead use a new typedef KIO::ProcessId. This is needed for Windows where pid_t doesn't exist and Q_PID is actually a pointer to a structure Diffs - src/core/global.h 7814a52c9656719d301ebd012434a53491ffe159 src/core/idleslave.h d3e7add4460d38212858666afa22deb3e5ef8687 src/core/idleslave.cpp 31d145e5d5e0c4205eb7deec6de7677061bb8a25 src/core/scheduler.h 593174e770d24c6ef813d91dfb6d6ed716bba4f9 src/core/scheduler.cpp 5a4ad310e34587249e0b50c67e011d516858e130 src/core/slave.h e4a443d191faefa535072d3821bb84cb6ab55909 src/core/slave.cpp 404a0c20f57e7c2badd7ac9c1294224d59960f7c src/core/slavebase.cpp a32c6b6ad2b803e24c1db75321aefb916678c03d src/core/slaveinterface.h 1ee7c1aebbf29271da605f33a5587974a4e71a5b src/core/slaveinterface.cpp f8d7d52864ab6dd11316a1252fbd1e372f7f9ee2 src/widgets/krun.cpp 34708cc5a4b4cf3627deb750020104dd3593ef92 src/widgets/krun_p.h cf44e327e6d5bac0fa69b989bd6036380fc5180c Diff: https://git.reviewboard.kde.org/r/116699/diff/ Testing --- tests still pass Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116740: Fix kdeglobals location in apidox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116740/#review52738 --- This review has been submitted with commit 58745952d1d2d27d5de4708200a8e74c70652f31 by Alex Merry to branch master. - Commit Hook On March 11, 2014, 7:46 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116740/ --- (Updated March 11, 2014, 7:46 p.m.) Review request for KDE Frameworks. Repository: kxmlgui Description --- Fix kdeglobals location in apidox Diffs - src/kcheckaccelerators.h 7ce96430b584543bf853a0595db93c1a0a569e59 Diff: https://git.reviewboard.kde.org/r/116740/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Update Meeting Minutes 2014-w11
On 12/03/14 07:56, Kevin Ottens wrote: Hello, On Tuesday 11 March 2014 22:19:06 David Gil Oliva wrote: 2014-03-11 16:33 GMT+01:00 Kevin Ottens er...@kde.org: Why is that? Are we in a hurry? Is the work in all the frameworks so advanced that we can advance the tagging? It just a question of David's availability. BTW, is it allowed to deprecate methods and add new methods from beta1 on? (please forgive my ignorance, I didn't find out what SIC change means). Yes, but don't break source compatibility one bit. Specifically, if you have to change any code that *uses* the frameworks in order to make it compile and behave as it did before (or, at least, as it was expected to behave), that's a source-incompatible (SIC) change. So, for example, replacing ints with enums is SIC, because downstream code may have to insert or remove casts. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: src/kcompletionbox.h, line 228 https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228 I wouldn't leave the implementation here. Move it to the .cpp file, this way it can be changed in the future, if it's required for some reason. Also there's a typo in the method name. David Gil Oliva wrote: Alex Merry inlined deprecated methods in https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way to go... Well, there's a balance to be struck: putting them in the header ensures there is no runtime cost to programs that don't use the deprecated methods, as the code should be optimised away, that the library is always binary-compatible even if you compile it with deprecated code disabled (*_NO_DEPRECATED) and makes the header code document how to replace existing calls. The downsides are an inability to fix the code later and an inability to access members of a private d-pointer class. Neither of those are an issue here, as we're just renaming the method. tl;dr: I disagree with Aleix, and think it should stay in the header. Oh, and Aleix: could you please select the whole method when you're doing a comment like this, rather than just the first line? Otherwise it's a pain to see what you're referring to. Thanks :-) On March 11, 2014, 11:17 p.m., David Gil Oliva wrote: Have you looked through the uses of the un-slotted methods? (lxr.kde.org). Maybe there's a reason for that... :/ David Gil Oliva wrote: Maybe I'm totally wrong, but I can't imagine any way that a getter can be useful as a slot. connect (widget, SLOT(valueChanged(), completionBox, SIGNAL(isTabHandling()); Does it make sense?¿?:-/ Non-void slots are only useful if they work as a slot (ie: have some sort of side-effect) and might be called directly or with QMetaObject::invokeMethod(). If the method is const (like a getter), there's no point having it a slot at all; if you want to be able to use it with invokeMethod, you can just make it Q_INVOKABLE. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116756: Remove Class Members link. It's broken and not useful.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116756/#review52741 --- Ship it! Ship It! - Alex Merry On March 12, 2014, 9:26 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116756/ --- (Updated March 12, 2014, 9:26 a.m.) Review request for KDE Frameworks and Aurélien Gâteau. Repository: kapidox Description --- Remove Class Members link. It's broken and not useful. Diffs - src/kapidox/__init__.py 32fa91b Diff: https://git.reviewboard.kde.org/r/116756/diff/ Testing --- Generated doc: no Class Members link anymore. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52740 --- Looks sensible to me, but I'll let Aleix reply. src/kcompletionbox.h https://git.reviewboard.kde.org/r/116747/#comment37196 If you change the parameter name, you have to change the docs to match src/kcompletionbox.h https://git.reviewboard.kde.org/r/116747/#comment37195 KCOMPLETION_NO_DEPRECATED - Alex Merry On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116758: Remove kcfg preprocessing script
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116758/ --- Review request for KDE Frameworks and Aurélien Gâteau. Repository: kapidox Description --- Remove kcfg preprocessing script The shell script does not make sense for the new kapidox implementation. Diffs - src/doxygen-preprocess-kcfg.sh 567a248e248f1bfdd66ec25cf547f03932f5f79a Diff: https://git.reviewboard.kde.org/r/116758/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116759: Update kde4 references in comments and apidox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116759/ --- Review request for KDE Frameworks. Repository: kio Description --- Update kde4 references in comments and apidox Diffs - autotests/kcookiejar/cookie.test efb55b7260c71895cca511a36b9b6b14d058e2b6 src/widgets/kfileitemactions.h 6ed9c4c91efdf18eca0575b4b29ff4cda3d5882c src/widgets/thumbcreator.h 384099d04bb696df3d0eea9ff5e07b52a495f4c5 Diff: https://git.reviewboard.kde.org/r/116759/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116758: Remove kcfg preprocessing script
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116758/#review52745 --- Ship it! Ship It! - Aurélien Gâteau On March 12, 2014, 1:49 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116758/ --- (Updated March 12, 2014, 1:49 p.m.) Review request for KDE Frameworks and Aurélien Gâteau. Repository: kapidox Description --- Remove kcfg preprocessing script The shell script does not make sense for the new kapidox implementation. Diffs - src/doxygen-preprocess-kcfg.sh 567a248e248f1bfdd66ec25cf547f03932f5f79a Diff: https://git.reviewboard.kde.org/r/116758/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116758: Remove kcfg preprocessing script
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116758/#review52746 --- This review has been submitted with commit 0e150c6112aab97db550191237036f98b138e4d4 by Alex Merry to branch master. - Commit Hook On March 12, 2014, 12:49 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116758/ --- (Updated March 12, 2014, 12:49 p.m.) Review request for KDE Frameworks and Aurélien Gâteau. Repository: kapidox Description --- Remove kcfg preprocessing script The shell script does not make sense for the new kapidox implementation. Diffs - src/doxygen-preprocess-kcfg.sh 567a248e248f1bfdd66ec25cf547f03932f5f79a Diff: https://git.reviewboard.kde.org/r/116758/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: src/kcompletionbox.h, line 228 https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228 I wouldn't leave the implementation here. Move it to the .cpp file, this way it can be changed in the future, if it's required for some reason. Also there's a typo in the method name. David Gil Oliva wrote: Alex Merry inlined deprecated methods in https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way to go... Alex Merry wrote: Well, there's a balance to be struck: putting them in the header ensures there is no runtime cost to programs that don't use the deprecated methods, as the code should be optimised away, that the library is always binary-compatible even if you compile it with deprecated code disabled (*_NO_DEPRECATED) and makes the header code document how to replace existing calls. The downsides are an inability to fix the code later and an inability to access members of a private d-pointer class. Neither of those are an issue here, as we're just renaming the method. tl;dr: I disagree with Aleix, and think it should stay in the header. Oh, and Aleix: could you please select the whole method when you're doing a comment like this, rather than just the first line? Otherwise it's a pain to see what you're referring to. Thanks :-) Well, I wouldn't bother about runtime penalty given that it's deprecated and we shouldn't be using it anyway. Also we can't make assumptions on how things are going to be optimized. But it's ok, I don't think it's worth discussing further, I doubt this is going to be a problem in the future. On March 11, 2014, 11:17 p.m., David Gil Oliva wrote: Have you looked through the uses of the un-slotted methods? (lxr.kde.org). Maybe there's a reason for that... :/ David Gil Oliva wrote: Maybe I'm totally wrong, but I can't imagine any way that a getter can be useful as a slot. connect (widget, SLOT(valueChanged(), completionBox, SIGNAL(isTabHandling()); Does it make sense?¿?:-/ Alex Merry wrote: Non-void slots are only useful if they work as a slot (ie: have some sort of side-effect) and might be called directly or with QMetaObject::invokeMethod(). If the method is const (like a getter), there's no point having it a slot at all; if you want to be able to use it with invokeMethod, you can just make it Q_INVOKABLE. Well, having const methods as slots doesn't make sense indeed, if it's not for exposing on the meta object system, that's why I said you could do a fast lxr. I just did, didn't find anything relevant. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116761: Remove KServiceGroup::baseGroup, deprecated since kdelibs 4.3.0
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116761/ --- Review request for KDE Frameworks. Repository: kservice Description --- Remove KServiceGroup::baseGroup, deprecated since kdelibs 4.3.0 Diffs - src/services/kservicegroup.h 00490e52dcef01740252fb59f27a558d3a2c6837 src/services/kservicegroup.cpp 6dec66997737e6617c13a6def5e06835beb6ce72 Diff: https://git.reviewboard.kde.org/r/116761/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: src/kcompletionbox.h, line 228 https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228 I wouldn't leave the implementation here. Move it to the .cpp file, this way it can be changed in the future, if it's required for some reason. Also there's a typo in the method name. David Gil Oliva wrote: Alex Merry inlined deprecated methods in https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way to go... Alex Merry wrote: Well, there's a balance to be struck: putting them in the header ensures there is no runtime cost to programs that don't use the deprecated methods, as the code should be optimised away, that the library is always binary-compatible even if you compile it with deprecated code disabled (*_NO_DEPRECATED) and makes the header code document how to replace existing calls. The downsides are an inability to fix the code later and an inability to access members of a private d-pointer class. Neither of those are an issue here, as we're just renaming the method. tl;dr: I disagree with Aleix, and think it should stay in the header. Oh, and Aleix: could you please select the whole method when you're doing a comment like this, rather than just the first line? Otherwise it's a pain to see what you're referring to. Thanks :-) Aleix Pol Gonzalez wrote: Well, I wouldn't bother about runtime penalty given that it's deprecated and we shouldn't be using it anyway. Also we can't make assumptions on how things are going to be optimized. But it's ok, I don't think it's worth discussing further, I doubt this is going to be a problem in the future. I mean the runtime penalty for things that *don't* use it. If it's header-only, it doesn't go in the library, so there is no load-time symbol-lookup penalty, and no code-size penalty. Admittedly, both of those are probably negligible... - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116762: Remove new in kde4 comment
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/ --- Review request for KDE Frameworks. Repository: kservice Description --- Remove new in kde4 comment Diffs - src/services/kservicetypeprofile.cpp 908e76599c553a48ea509695f1d8b83be98fee1a Diff: https://git.reviewboard.kde.org/r/116762/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116762: Remove new in kde4 comment
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/#review52753 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 12, 2014, 1:46 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/ --- (Updated March 12, 2014, 1:46 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Remove new in kde4 comment Diffs - src/services/kservicetypeprofile.cpp 908e76599c553a48ea509695f1d8b83be98fee1a Diff: https://git.reviewboard.kde.org/r/116762/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116766: Use a desktop file we might find for testing KServiceActions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116766/ --- Review request for KDE Frameworks. Repository: kservice Description --- Use a desktop file we might find for testing KServiceActions The screensaver stuff isn't ported to frameworks 5 yet, so looking for those desktop files won't work. This uses something that might actually be installed instead. Diffs - autotests/kservicetest.h aa1c691c75c9e5b6903bcbf6e2dc95809ee1ce21 autotests/kservicetest.cpp 711fb9b649e580ad474b0cdecd26dcdbfdc302a2 Diff: https://git.reviewboard.kde.org/r/116766/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116762: Remove new in kde4 comment
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/ --- (Updated March 12, 2014, 2:47 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kservice Description --- Remove new in kde4 comment Diffs - src/services/kservicetypeprofile.cpp 908e76599c553a48ea509695f1d8b83be98fee1a Diff: https://git.reviewboard.kde.org/r/116762/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116762: Remove new in kde4 comment
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/#review52762 --- This review has been submitted with commit 581e328460538203b588db01430fd04a259e29d5 by Alex Merry to branch master. - Commit Hook On March 12, 2014, 1:46 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116762/ --- (Updated March 12, 2014, 1:46 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Remove new in kde4 comment Diffs - src/services/kservicetypeprofile.cpp 908e76599c553a48ea509695f1d8b83be98fee1a Diff: https://git.reviewboard.kde.org/r/116762/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116735: Split autotests into their own directory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116735/ --- Review request for KDE Frameworks. Repository: knewstuff Description --- Split autotests into their own directory Tests are still not built, as they need porting, but the build infrastructure should be ready for new or ported tests. Diffs - CMakeLists.txt 4edb7248b71a62deaf81e6a2a9cada9a03bb1c16 autotests/CMakeLists.txt PRE-CREATION tests/CMakeLists.txt 14bcf85fe128874ae396d5dba1263dd378baf9cd tests/knewstuffauthortest.cpp tests/knewstuffentrytest.cpp tests/testTranslatable.cpp Diff: https://git.reviewboard.kde.org/r/116735/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116761: Remove KServiceGroup::baseGroup, deprecated since kdelibs 4.3.0
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116761/ --- (Updated March 12, 2014, 2:48 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kservice Description --- Remove KServiceGroup::baseGroup, deprecated since kdelibs 4.3.0 Diffs - src/services/kservicegroup.h 00490e52dcef01740252fb59f27a558d3a2c6837 src/services/kservicegroup.cpp 6dec66997737e6617c13a6def5e06835beb6ce72 Diff: https://git.reviewboard.kde.org/r/116761/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116767: Clean up CMake code from pre-splitting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/ --- Review request for KDE Frameworks. Repository: kservice Description --- Clean up CMake code from pre-splitting Remove code that was for building as part of kdelibs, add feature_summary and respect the option to disable tests. Diffs - CMakeLists.txt 4d08bb97a569488849504e04078a71995a1fc53b Diff: https://git.reviewboard.kde.org/r/116767/diff/ Testing --- Configures, builds and installs. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116767: Clean up CMake code from pre-splitting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/#review52772 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 12, 2014, 2:57 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/ --- (Updated March 12, 2014, 2:57 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Clean up CMake code from pre-splitting Remove code that was for building as part of kdelibs, add feature_summary and respect the option to disable tests. Diffs - CMakeLists.txt 4d08bb97a569488849504e04078a71995a1fc53b Diff: https://git.reviewboard.kde.org/r/116767/diff/ Testing --- Configures, builds and installs. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: KCrash: remove usage of strlcpy
On March 1, 2014, 4:17 p.m., David Faure wrote:
Hmm, this might be equivalent, but all it means is that the orig code was
wrong.
We should not make any memory allocations within the crash handler.
So we should instead store the startup id as a const char* somewhere and
use strlcpy.
Unless we can make sure that the call to startupId() will always just
return an existing QByteArray - but looking at the code, this doesn't seem
to be the case.
Alex Merry wrote:
Hrm... I think we're actually querying the wrong place anyway. We should
be asking the xcb platform plugin, since that's what actually handles the
startup notifications, since the demise of KApplication (perhaps that part of
KStartupInfo's functionality should be stripped out?).
Note that the platform plugin does always just return an existing
QByteArray, so that should be fine. Which is good, because taking our own
copy outside the handler would not work, as we would need to know when it was
cleared.
Alex Merry wrote:
Actually, you don't get access to the QByteArray. You could do something
like
QPlatformNativeInterface *platform =
QGuiApplication::platformNativeInterface();
const char * startupId = reinterpret_castchar
*(platform-nativeResourceForIntegration(QByteArrayLiteral(startupid)));
if (startupId *startupId) {
argv[i++] = --startupid;
argv[i++] = startupId;
}
Since we're in the crash handler, is it safe to assume that the rest of
the application is stopped, and so that string will never disappear from
underneath us?
Alexander Richardson wrote:
I'll update the review to use this solution if someone else can confirm
that this is safe. Even in multithreaded environments? Does the crash handler
stop all threads?
I think so yes, David could you confirm?
- Kevin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116087/#review51440
---
On Feb. 26, 2014, 5 p.m., Alexander Richardson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116087/
---
(Updated Feb. 26, 2014, 5 p.m.)
Review request for KDE Frameworks.
Repository: kcrash
Description
---
remove usage of strlcpy
That copy was actually unnecessary, incrementing the refcount on
QByteArray and then calling constData() is enough
Diffs
-
src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f
src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d
src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e
src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8
Diff: https://git.reviewboard.kde.org/r/116087/diff/
Testing
---
Compiles
Thanks,
Alexander Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: KCrash: remove usage of strlcpy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 12, 2014, 3:26 p.m.) Review request for KDE Frameworks and David Faure. Changes --- Pending question for David. Repository: kcrash Description --- remove usage of strlcpy That copy was actually unnecessary, incrementing the refcount on QByteArray and then calling constData() is enough Diffs - src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115916: Fix MSVC build of kprintpreview_test
On March 4, 2014, 8:11 p.m., Kevin Ottens wrote: Ship It! Any news? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115916/#review51930 --- On Feb. 20, 2014, 4:41 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115916/ --- (Updated Feb. 20, 2014, 4:41 p.m.) Review request for KDE Frameworks. Repository: kprintutils Description --- Fix MSVC build of kprintpreview_test I am not sure whether this is the best solution, but I don't want to make the Linux code less efficient. However it is a test application so it probably doesn't matter... Diffs - tests/kprintpreview_test.cpp 79cac037ab38bce89b97e4ede58eb58d821b25f3 Diff: https://git.reviewboard.kde.org/r/115916/diff/ Testing --- Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115977: RFC: Install KArchive as Mac OS X Framework
On March 4, 2014, 8:28 p.m., Kevin Ottens wrote: src/karchive.h, line 32 https://git.reviewboard.kde.org/r/115977/diff/1/?file=245721#file245721line32 Hm, why the change to for the includes? We try to stick to in public headers. Any news? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115977/#review51936 --- On Feb. 23, 2014, 7:15 p.m., Harald Fernengel wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115977/ --- (Updated Feb. 23, 2014, 7:15 p.m.) Review request for KDE Frameworks. Repository: karchive Description --- Change CMakeLists.txt to create Mac OS X frameworks Diffs - CMakeLists.txt f5dc644fdba13e29c94940c77d628e92e0759787 src/CMakeLists.txt 53e97284cab199f5eb75aa276adfadc18d677682 src/karchive.h d4209cf334190dda735fcb4687fa102a4e7a73cd src/karchivedirectory.h 60225d0be9fc2e28ff2b998dcc8fb28512c6e3cd src/karchiveentry.h aad6840ee0dc22e5760ddda99ce975a1d9a9dc92 src/karchivefile.h c7d2e0e0735f75a8e490082aa8598fd08206a998 src/ktar.h 4bca89884e646ffae90aa1a9e15a985e998e843f Diff: https://git.reviewboard.kde.org/r/115977/diff/ Testing --- 'make install' on Mac OS X Thanks, Harald Fernengel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116044: In kstyle: Use the iconSize from mainToolbar
On Feb. 25, 2014, 12:10 p.m., David Faure wrote: The part of the description that says if accepted will modify kstyle as well doesn't really make sense anymore (to fix if it's in your commit log too). The bit I'm not sure about is: using MainToolbar icon style everywhere ... how does that take care of other toolbars then? The idea (long ago) was to be able to have (large) icons and text in the main toolbar, and (small) icons only in other toolbars. Is that idea dropped, or handled elsewhere? Hugo Pereira Da Costa wrote: The bit I'm not sure about is: using MainToolbar icon style everywhere ... how does that take care of other toolbars then? Thing is, old code was treating 'main ToolBar' as 'other toolbars'. New one (iiur) treats 'other toolbars' as 'main toolbar'. The latter is as 'incomplete' as the former, but more consistent. Not sure where exactly the distinction between 'main' and 'all' toolbars is performed. Alex ? Is it kapplication ? (as opposed to Qt only, which has no such distinction) ? David Faure wrote: I know consistency is better than inconsistency, but still, while looking at this we might as well make it correct, too :) The logic used to be in KToolBar, based on objectName == mainToolBar (yes, you could say that's ugly and fragile, but since that comes from the kxmlgui ui_standards.rc file, it actually happens automatically in all apps). I'm not sure what's the plan for making this work in the QToolBar+KF5 world. But if nothing else has been planned already, kstyle and/or platformtheme [I'm not sure why they both handle toolbar icon sizes] can take care of checking the objectName too, given a pointer to the toolbar. Kevin Ottens wrote: The style is indeed able to have that pointer to the toolbar, so it could be a simple if there. Hugo, any news? Still something blocking you? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116044/#review50807 --- On Feb. 25, 2014, 12:34 p.m., Hugo Pereira Da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116044/ --- (Updated Feb. 25, 2014, 12:34 p.m.) Review request for KDE Frameworks and Àlex Fiestas. Repository: frameworkintegration Description --- This is a spinoff of https://git.reviewboard.kde.org/r/112335/ originally from afiestas Copying his own words: In the qplatformplugin we use information from MainToolbar (which makes sense) but in the styles we use information from Toolbar. This unify both by using MainToolbar in styles Code has been removed from oxygen now that it derives from KStyle again Diffs - src/kstyle/kstyle.cpp c0528b3 Diff: https://git.reviewboard.kde.org/r/116044/diff/ Testing --- Thanks, Hugo Pereira Da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116037: change entities to reflect new branding
On March 4, 2014, 8:48 p.m., Kevin Ottens wrote: Ship It! Any news? Anything blocking this patch from being committed? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116037/#review51948 --- On Feb. 25, 2014, 1:02 a.m., T.C. Hollingsworth wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116037/ --- (Updated Feb. 25, 2014, 1:02 a.m.) Review request for Documentation, KDE Frameworks, Luigi Toscano, Burkhard Lück, and Yuri Chornoivan. Repository: kdoctools Description --- -kde; is no longer semantically identified as an acronym, but instead as an organization -new translatable entities for KDE Frameworks are added -new translatable entities for the KDE SC are added -new translatable entities for the various incarnations of Plasma are added with this I can finally kill the temporary entities that have been in kde-runtime.git/doc/fundamentals for over a year now. :-) Diffs - src/customization/en/user.entities 47bfe0d src/customization/entities/general.entities 183fbc9 Diff: https://git.reviewboard.kde.org/r/116037/diff/ Testing --- kdoctools still builds, kate docs still meinproc5 successfully Thanks, T.C. Hollingsworth ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115717: Do not require to have a DISPLAY env variable if WAYLAND_DISPLAY is set
On Feb. 19, 2014, 2:07 p.m., Alex Merry wrote: I don't think papering over the X11-ness of kdesu like this is the right approach. Of course, what this framework really needs is a test app; maybe a simple port of the kdesu app from kde-runtime? Kevin Ottens wrote: This kind of papering over can only be temporary indeed. Just wondering if Martin has the possibility to clean that up better at that stage? As for the test app: hell yes, definitely needed. Martin Gräßlin wrote: well yes if there's a testapp and that works I can implement it properly. But IIRC kdesu never worked on my setup (not having a password for root). Kevin Ottens wrote: You can use it for other users than root you know. ;-) Martin Gräßlin wrote: no, I didn't and which other users? ;-) Kevin Ottens wrote: My point is that if you need to test it's trivial to add a temporary user... So what do we do of this patch? Any chance of a test app? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115717/#review50240 --- On Feb. 13, 2014, 9:41 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115717/ --- (Updated Feb. 13, 2014, 9:41 a.m.) Review request for KDE Frameworks. Repository: kdesu Description --- Do not require to have a DISPLAY env variable if WAYLAND_DISPLAY is set If kdesu is compiled with X11 it required the DISPLAY variable to be set. This is no longer correct as it might have been compiled with X11 but is run on Wayland. Thus the code checks now also for WAYLAND_DISPLAY in the HAVE_X11 ifdef blocks. The Wayland support should become more complete, I do not know how it behaves if we compile without X11 support. Unfortunately there are no autotests and no test applications which one could use. Diffs - src/client.cpp 91bfd78fbca6e5d8d365d924c0260087e3937948 src/kcookie.cpp 59448351696c503b34b7507e9c3fa8efc53139f9 Diff: https://git.reviewboard.kde.org/r/115717/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116684: Improve API in KCompletionBase
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116684/#review52783
---
src/kcompletionbase.h
https://git.reviewboard.kde.org/r/116684/#comment37206
Hm, shouldn't the QMap include be moved in the header? That's the only
partially defined type at play I could think of here.
- Kevin Ottens
On March 9, 2014, 11:06 p.m., David Gil Oliva wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116684/
---
(Updated March 9, 2014, 11:06 p.m.)
Review request for KDE Frameworks.
Repository: kcompletion
Description
---
Improve API in KCompletionBase
getKeyBindings - keyBindingMap
getKeyBinding - keyBinding
NOTICE: The following part is commented out because it gives me errors and I
need some help to find out what actually happens:
/**
* @deprecated since 5.0, use keyBindingMap instead
*/
#ifndef KDE_NO_DEPRECATED
KCOMPLETION_DEPRECATED KeyBindingMap getKeyBindings() const
{
return keyBindingMap();
}
#endif
Error:
In file included from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21:0:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:
In member function ‘KCompletionBase::KeyBindingMap
KCompletionBase::getKeyBindings() const’:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5:
error: return type ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’ is incomplete
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30:
error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka
class QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
In file included from
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0,
from
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:23,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21:
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error:
declaration of ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
In file included from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:27:0,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:
In member function ‘KCompletionBase::KeyBindingMap
KCompletionBase::getKeyBindings() const’:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5:
error: return type ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’ is incomplete
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30:
error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka
class QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
In file included from
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0,
from
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:25,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23:
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error:
declaration of ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
make[2]: *** [src/CMakeFiles/KF5Completion.dir/kcompletionbase.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs
Diffs
-
src/kcompletionbase.h 1dedd2d
src/kcompletionbase.cpp 1e251d1
src/klineedit.cpp ae15093
Diff: https://git.reviewboard.kde.org/r/116684/diff/
Testing
---
It compiles and (auto)tests pass without the conflicting part
Re: Review Request 115918: Fix kservice_desktop_to_json for Visual Studio
On March 4, 2014, 8:45 p.m., Kevin Ottens wrote: And I agree with Aurélien, a bug should be filed and Stephen involved in that issue. Stephen Kelly wrote: Please provide a minimal testcase. The feature is unit tested in cmake. If it's broken, it needs to be fixed soon (before the final 3.0 release). Alexander Richardson wrote: I'll try to reproduce it with a minimal test case, however I am away until Sunday, so it will be a while. Stephen Kelly wrote: I've pushed a fix to the cmake next branch. Please test it instead of committing this patch. Another point of note is that you shouldn't read the LOCATION property from the target. You get a policy warning about that. Use the target name directly instead. Alexander Richardson wrote: I can confirm that using cmake next branch fixes the issue. How do I check that the currently used CMake version contains that fix? No idea, anyone? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115918/#review51946 --- On March 4, 2014, 8:45 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115918/ --- (Updated March 4, 2014, 8:45 p.m.) Review request for KDE Frameworks and Stephen Kelly. Repository: kservice Description --- Fix kservice_desktop_to_json for Visual Studio The CMake generator for Visual Studio cannot handle the new build-time kservice_desktop_to_json macro - fall back to configure-time generation Diffs - KF5ServiceMacros.cmake f70a185f4cd48293cb9f1e2ca4cf13defaf2aec3 Diff: https://git.reviewboard.kde.org/r/115918/diff/ Testing --- ktexteditor fails to build before this patch (because moc can't find the .json file), with the patch it compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116670: Use absolute paths to refer to entities/elements defined in kdoctools
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116670/#review52788 --- Ship it! Ship It! - Kevin Ottens On March 9, 2014, 2:39 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116670/ --- (Updated March 9, 2014, 2:39 p.m.) Review request for KDE Frameworks, kdewin, Alexander Richardson, and Harald Fernengel. Repository: kde4support Description --- The processing of documentation still using DocBookXML 4.2, which depends on kde4support, currently fails on our Jenkins instance where each module has a different installation prefix with an error like: --- Generating kate.1 file:///usr/share/xml/docbook/schema/dtd/4.2/dbpoolx.mod:215: warning: failed to load external entity file:///srv/jenkins/install/linux/x86_64/g++/kf5- qt5/frameworks/kde4support/inst/share/kdoctools5/customization/dtd/rdbpool.elements %rdbpool; ^ Entity: line 1: %rdbpool; ^ --- and similar for the other element/entity files referred. This does not happen in the normal installation because in that case those files will be found in the same directory ($prefix/share/kdoctools5/customization/dtd) and the relative path will work. The attached patch change the paths in the compatibiliy DTD to be absolute path instead. I have to question about this solution (kdewin group): will it work on windows? I guess so, as the directory returned by KDOCTOOLS_CUSTOMIZATION_DIR is set by KF5DocToolsConfig.cmake using KDOCTOOLS_DATA_INSTALL_DIR which uses PACKAGE_PREFIX_DIR, but I'm not totally sure. Similar question for MacOSX. Diffs - src/customization/dtd/kdex.dtd.cmake 1f75dd9 src/CMakeLists.txt d436846 Diff: https://git.reviewboard.kde.org/r/116670/diff/ Testing --- It compiles, and a module whose documentation still uses the old DTD (oktate) compiles correctly. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116725: Remove out-of-date README
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116725/#review52793 --- Ship it! Ship It! - Kevin Ottens On March 11, 2014, 4:33 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116725/ --- (Updated March 11, 2014, 4:33 p.m.) Review request for KDE Frameworks. Repository: kdesignerplugin Description --- Remove out-of-date README All the information that is still applicable can be found in README.md. Diffs - src/README cbf079488d9961a718201373fd4bb3483b2f3bf8 Diff: https://git.reviewboard.kde.org/r/116725/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116724: Replace comment that refers to removed code
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116724/#review52791 --- Ship it! Ship It! - Kevin Ottens On March 11, 2014, 4:28 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116724/ --- (Updated March 11, 2014, 4:28 p.m.) Review request for KDE Frameworks. Repository: kdeclarative Description --- Replace comment that refers to removed code Diffs - src/kdeclarative.cpp c074efb89a35f22cdc1ae7823749555623957755 Diff: https://git.reviewboard.kde.org/r/116724/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116727: Remove obsolete code
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116727/#review52795 --- Ship it! Ship It! - Kevin Ottens On March 11, 2014, 4:50 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116727/ --- (Updated March 11, 2014, 4:50 p.m.) Review request for KDE Frameworks. Repository: kinit Description --- Remove obsolete code This code had #if 0 around it through all of KDE 4, so it clearly wasn't that important. Diffs - src/kdeinit/kinit.cpp c1010dd7f8980ae8b4b456a41d9094a16aca62a6 Diff: https://git.reviewboard.kde.org/r/116727/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116739: Update commented-out cmake code to new macros
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116739/#review52797 --- Ship it! Ship It! - Kevin Ottens On March 11, 2014, 7:42 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116739/ --- (Updated March 11, 2014, 7:42 p.m.) Review request for KDE Frameworks. Repository: solid Description --- Update commented-out cmake code to new macros This test is also disabled in kdelibs4; it appears to use a non-existant mock D-Bus service. At least this way the CMake code is ready to be re-enabled if and when the test is fixed up. Diffs - tests/CMakeLists.txt 59337085e005c9ad93f490e269dfcdcae8628d78 Diff: https://git.reviewboard.kde.org/r/116739/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/#review52798 --- Ship it! Ship It! - Kevin Ottens On March 10, 2014, 5:46 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 10, 2014, 5:46 p.m.) Review request for Documentation, KDE Frameworks and David Faure. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116760: Remove kcookiescfg update scripts
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116760/ --- (Updated March 12, 2014, 3:44 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- Am I right about this? Remove kcookiescfg update scripts kf5 stores configuration in different locations to kde4, so there should be nothing to update. Diffs - src/ioslaves/http/kcookiejar/CMakeLists.txt 2bb4cc232b7011d38033da10bb312a12215cb29d src/ioslaves/http/kcookiejar/kcookiescfg.pl 7fef01d5c208a00987b3594411ce06799457 src/ioslaves/http/kcookiejar/kcookiescfg.upd 745f545090875d5d24e1e6bb397af9926704350f Diff: https://git.reviewboard.kde.org/r/116760/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116760: Remove kcookiescfg update scripts
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116760/#review52801 --- I think it's fine indeed. Let's see what David think about it. - Kevin Ottens On March 12, 2014, 3:44 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116760/ --- (Updated March 12, 2014, 3:44 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- Am I right about this? Remove kcookiescfg update scripts kf5 stores configuration in different locations to kde4, so there should be nothing to update. Diffs - src/ioslaves/http/kcookiejar/CMakeLists.txt 2bb4cc232b7011d38033da10bb312a12215cb29d src/ioslaves/http/kcookiejar/kcookiescfg.pl 7fef01d5c208a00987b3594411ce06799457 src/ioslaves/http/kcookiejar/kcookiescfg.upd 745f545090875d5d24e1e6bb397af9926704350f Diff: https://git.reviewboard.kde.org/r/116760/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116766: Use a desktop file we might find for testing KServiceActions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116766/ --- (Updated March 12, 2014, 3:47 p.m.) Review request for KDE Frameworks and David Faure. Repository: kservice Description --- Use a desktop file we might find for testing KServiceActions The screensaver stuff isn't ported to frameworks 5 yet, so looking for those desktop files won't work. This uses something that might actually be installed instead. Diffs - autotests/kservicetest.h aa1c691c75c9e5b6903bcbf6e2dc95809ee1ce21 autotests/kservicetest.cpp 711fb9b649e580ad474b0cdecd26dcdbfdc302a2 Diff: https://git.reviewboard.kde.org/r/116766/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116766: Use a desktop file we might find for testing KServiceActions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116766/#review52804 --- Looks fine to me. - Kevin Ottens On March 12, 2014, 3:47 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116766/ --- (Updated March 12, 2014, 3:47 p.m.) Review request for KDE Frameworks and David Faure. Repository: kservice Description --- Use a desktop file we might find for testing KServiceActions The screensaver stuff isn't ported to frameworks 5 yet, so looking for those desktop files won't work. This uses something that might actually be installed instead. Diffs - autotests/kservicetest.h aa1c691c75c9e5b6903bcbf6e2dc95809ee1ce21 autotests/kservicetest.cpp 711fb9b649e580ad474b0cdecd26dcdbfdc302a2 Diff: https://git.reviewboard.kde.org/r/116766/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116735: Split autotests into their own directory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116735/#review52806 --- Ship it! Ship It! - Kevin Ottens On March 12, 2014, 2:51 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116735/ --- (Updated March 12, 2014, 2:51 p.m.) Review request for KDE Frameworks. Repository: knewstuff Description --- Split autotests into their own directory Tests are still not built, as they need porting, but the build infrastructure should be ready for new or ported tests. Diffs - CMakeLists.txt 4edb7248b71a62deaf81e6a2a9cada9a03bb1c16 autotests/CMakeLists.txt PRE-CREATION tests/CMakeLists.txt 14bcf85fe128874ae396d5dba1263dd378baf9cd tests/knewstuffauthortest.cpp tests/knewstuffentrytest.cpp tests/testTranslatable.cpp Diff: https://git.reviewboard.kde.org/r/116735/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116767: Clean up CMake code from pre-splitting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/#review52807 --- Ship it! Ship It! - Kevin Ottens On March 12, 2014, 2:57 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/ --- (Updated March 12, 2014, 2:57 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Clean up CMake code from pre-splitting Remove code that was for building as part of kdelibs, add feature_summary and respect the option to disable tests. Diffs - CMakeLists.txt 4d08bb97a569488849504e04078a71995a1fc53b Diff: https://git.reviewboard.kde.org/r/116767/diff/ Testing --- Configures, builds and installs. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116768: Make our css more future-proof
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116768/ --- Review request for KDE Frameworks and Aurélien Gâteau. Repository: kapidox Description --- - Remove outdated files - Extend the doxygen.css file provided by Doxygen instead of replacing it - Trim kde.css to contain only necessary lines Diffs - src/kapidox/data/htmlresource/print.css 16818ba src/kapidox/data/htmlresource/tabs.css a61552a src/kapidox/data/htmlresource/kde.css c6c2d86 src/kapidox/data/header.html 109045e src/kapidox/data/htmlresource/flat.css e1db552 src/kapidox/__init__.py 3127357 src/kapidox/data/doxygen.css a0f924e Diff: https://git.reviewboard.kde.org/r/116768/diff/ Testing --- Regenerated doc for all frameworks using kgenframeworksapidox. Browsed around. Did not spot regressions. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: KCrash: remove usage of strlcpy
On March 1, 2014, 4:17 p.m., David Faure wrote:
Hmm, this might be equivalent, but all it means is that the orig code was
wrong.
We should not make any memory allocations within the crash handler.
So we should instead store the startup id as a const char* somewhere and
use strlcpy.
Unless we can make sure that the call to startupId() will always just
return an existing QByteArray - but looking at the code, this doesn't seem
to be the case.
Alex Merry wrote:
Hrm... I think we're actually querying the wrong place anyway. We should
be asking the xcb platform plugin, since that's what actually handles the
startup notifications, since the demise of KApplication (perhaps that part of
KStartupInfo's functionality should be stripped out?).
Note that the platform plugin does always just return an existing
QByteArray, so that should be fine. Which is good, because taking our own
copy outside the handler would not work, as we would need to know when it was
cleared.
Alex Merry wrote:
Actually, you don't get access to the QByteArray. You could do something
like
QPlatformNativeInterface *platform =
QGuiApplication::platformNativeInterface();
const char * startupId = reinterpret_castchar
*(platform-nativeResourceForIntegration(QByteArrayLiteral(startupid)));
if (startupId *startupId) {
argv[i++] = --startupid;
argv[i++] = startupId;
}
Since we're in the crash handler, is it safe to assume that the rest of
the application is stopped, and so that string will never disappear from
underneath us?
Alexander Richardson wrote:
I'll update the review to use this solution if someone else can confirm
that this is safe. Even in multithreaded environments? Does the crash handler
stop all threads?
Kevin Ottens wrote:
I think so yes, David could you confirm?
I'm not an expert on crash handling in a multithreaded application, but anyway,
threads do not call into the xcb plugin (it's not threadsafe, and it's a GUI
thing anyway). So yeah Alex Merry's suggestion sounds fine.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116087/#review51440
---
On March 12, 2014, 3:26 p.m., Alexander Richardson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116087/
---
(Updated March 12, 2014, 3:26 p.m.)
Review request for KDE Frameworks and David Faure.
Repository: kcrash
Description
---
remove usage of strlcpy
That copy was actually unnecessary, incrementing the refcount on
QByteArray and then calling constData() is enough
Diffs
-
src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f
src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d
src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e
src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8
Diff: https://git.reviewboard.kde.org/r/116087/diff/
Testing
---
Compiles
Thanks,
Alexander Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116724: Replace comment that refers to removed code
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116724/#review52820 --- This review has been submitted with commit 5e6b9bab04b572ccd38a628f2046dd8d9677a3f7 by Alex Merry to branch master. - Commit Hook On March 11, 2014, 4:28 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116724/ --- (Updated March 11, 2014, 4:28 p.m.) Review request for KDE Frameworks. Repository: kdeclarative Description --- Replace comment that refers to removed code Diffs - src/kdeclarative.cpp c074efb89a35f22cdc1ae7823749555623957755 Diff: https://git.reviewboard.kde.org/r/116724/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116725: Remove out-of-date README
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116725/ --- (Updated March 12, 2014, 4:48 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdesignerplugin Description --- Remove out-of-date README All the information that is still applicable can be found in README.md. Diffs - src/README cbf079488d9961a718201373fd4bb3483b2f3bf8 Diff: https://git.reviewboard.kde.org/r/116725/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116726: Remove unused CMakeLists.txt file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116726/ --- (Updated March 12, 2014, 4:49 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. Repository: khtml Description --- Remove unused CMakeLists.txt file Diffs - tests/pics/CMakeLists.txt 3d2f8d5286205336f1ea6f31714c226b0d7a142f Diff: https://git.reviewboard.kde.org/r/116726/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116726: Remove unused CMakeLists.txt file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116726/#review52822 --- This review has been submitted with commit 7f34d1ef2f81fd0991a7778dc2960a11b5fd97fb by Alex Merry to branch master. - Commit Hook On March 11, 2014, 4:40 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116726/ --- (Updated March 11, 2014, 4:40 p.m.) Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. Repository: khtml Description --- Remove unused CMakeLists.txt file Diffs - tests/pics/CMakeLists.txt 3d2f8d5286205336f1ea6f31714c226b0d7a142f Diff: https://git.reviewboard.kde.org/r/116726/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116727: Remove obsolete code
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116727/#review52823 --- This review has been submitted with commit 3f79b430e2b90a8c429cb9da39a642ba20bd05d9 by Alex Merry to branch master. - Commit Hook On March 11, 2014, 4:50 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116727/ --- (Updated March 11, 2014, 4:50 p.m.) Review request for KDE Frameworks. Repository: kinit Description --- Remove obsolete code This code had #if 0 around it through all of KDE 4, so it clearly wasn't that important. Diffs - src/kdeinit/kinit.cpp c1010dd7f8980ae8b4b456a41d9094a16aca62a6 Diff: https://git.reviewboard.kde.org/r/116727/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116727: Remove obsolete code
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116727/ --- (Updated March 12, 2014, 4:50 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kinit Description --- Remove obsolete code This code had #if 0 around it through all of KDE 4, so it clearly wasn't that important. Diffs - src/kdeinit/kinit.cpp c1010dd7f8980ae8b4b456a41d9094a16aca62a6 Diff: https://git.reviewboard.kde.org/r/116727/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116728: Remove no-longer-needed magic variable
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116728/#review52824 --- This review has been submitted with commit bdda6298b22b657a1d97008e49bd0243fe65d454 by Alex Merry to branch master. - Commit Hook On March 11, 2014, 4:50 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116728/ --- (Updated March 11, 2014, 4:50 p.m.) Review request for KDE Frameworks. Repository: kjs Description --- Remove no-longer-needed magic variable The manifest stuff is all gone for now, and won't return in the same form, so remove the variable with the special name and the comment about it. Diffs - src/kjs/CMakeLists.txt 491cf52ea7d3d8a6b138fb328fdfdeec61f6740a Diff: https://git.reviewboard.kde.org/r/116728/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116728: Remove no-longer-needed magic variable
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116728/ --- (Updated March 12, 2014, 4:51 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kjs Description --- Remove no-longer-needed magic variable The manifest stuff is all gone for now, and won't return in the same form, so remove the variable with the special name and the comment about it. Diffs - src/kjs/CMakeLists.txt 491cf52ea7d3d8a6b138fb328fdfdeec61f6740a Diff: https://git.reviewboard.kde.org/r/116728/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116739: Update commented-out cmake code to new macros
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116739/#review52826 --- This review has been submitted with commit 661971b0564dce58841019fce3fc1ed51984302d by Alex Merry to branch master. - Commit Hook On March 11, 2014, 7:42 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116739/ --- (Updated March 11, 2014, 7:42 p.m.) Review request for KDE Frameworks. Repository: solid Description --- Update commented-out cmake code to new macros This test is also disabled in kdelibs4; it appears to use a non-existant mock D-Bus service. At least this way the CMake code is ready to be re-enabled if and when the test is fixed up. Diffs - tests/CMakeLists.txt 59337085e005c9ad93f490e269dfcdcae8628d78 Diff: https://git.reviewboard.kde.org/r/116739/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116759: Update kde4 references in comments and apidox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116759/ --- (Updated March 12, 2014, 4:53 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kio Description --- Update kde4 references in comments and apidox Diffs - autotests/kcookiejar/cookie.test efb55b7260c71895cca511a36b9b6b14d058e2b6 src/widgets/kfileitemactions.h 6ed9c4c91efdf18eca0575b4b29ff4cda3d5882c src/widgets/thumbcreator.h 384099d04bb696df3d0eea9ff5e07b52a495f4c5 Diff: https://git.reviewboard.kde.org/r/116759/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116759: Update kde4 references in comments and apidox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116759/#review52827 --- This review has been submitted with commit 1527d6faeb7e9b79adc6f81b740fcea92e005ad7 by Alex Merry to branch master. - Commit Hook On March 12, 2014, 12:58 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116759/ --- (Updated March 12, 2014, 12:58 p.m.) Review request for KDE Frameworks. Repository: kio Description --- Update kde4 references in comments and apidox Diffs - autotests/kcookiejar/cookie.test efb55b7260c71895cca511a36b9b6b14d058e2b6 src/widgets/kfileitemactions.h 6ed9c4c91efdf18eca0575b4b29ff4cda3d5882c src/widgets/thumbcreator.h 384099d04bb696df3d0eea9ff5e07b52a495f4c5 Diff: https://git.reviewboard.kde.org/r/116759/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116725: Remove out-of-date README
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116725/#review52821 --- This review has been submitted with commit f348842a676a1b22f513d5ff14a9eebd8f6b230c by Alex Merry to branch master. - Commit Hook On March 11, 2014, 4:33 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116725/ --- (Updated March 11, 2014, 4:33 p.m.) Review request for KDE Frameworks. Repository: kdesignerplugin Description --- Remove out-of-date README All the information that is still applicable can be found in README.md. Diffs - src/README cbf079488d9961a718201373fd4bb3483b2f3bf8 Diff: https://git.reviewboard.kde.org/r/116725/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116735: Split autotests into their own directory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116735/#review52828 --- This review has been submitted with commit 64e14b39ad68f232cc2d2dd384801d8ac44ffd6e by Alex Merry to branch master. - Commit Hook On March 12, 2014, 2:51 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116735/ --- (Updated March 12, 2014, 2:51 p.m.) Review request for KDE Frameworks. Repository: knewstuff Description --- Split autotests into their own directory Tests are still not built, as they need porting, but the build infrastructure should be ready for new or ported tests. Diffs - CMakeLists.txt 4edb7248b71a62deaf81e6a2a9cada9a03bb1c16 autotests/CMakeLists.txt PRE-CREATION tests/CMakeLists.txt 14bcf85fe128874ae396d5dba1263dd378baf9cd tests/knewstuffauthortest.cpp tests/knewstuffentrytest.cpp tests/testTranslatable.cpp Diff: https://git.reviewboard.kde.org/r/116735/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116735: Split autotests into their own directory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116735/ --- (Updated March 12, 2014, 4:55 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: knewstuff Description --- Split autotests into their own directory Tests are still not built, as they need porting, but the build infrastructure should be ready for new or ported tests. Diffs - CMakeLists.txt 4edb7248b71a62deaf81e6a2a9cada9a03bb1c16 autotests/CMakeLists.txt PRE-CREATION tests/CMakeLists.txt 14bcf85fe128874ae396d5dba1263dd378baf9cd tests/knewstuffauthortest.cpp tests/knewstuffentrytest.cpp tests/testTranslatable.cpp Diff: https://git.reviewboard.kde.org/r/116735/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to stable : kio_master_qt5 #100
See http://build.kde.org/job/kio_master_qt5/100/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116767: Clean up CMake code from pre-splitting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/ --- (Updated March 12, 2014, 4:58 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kservice Description --- Clean up CMake code from pre-splitting Remove code that was for building as part of kdelibs, add feature_summary and respect the option to disable tests. Diffs - CMakeLists.txt 4d08bb97a569488849504e04078a71995a1fc53b Diff: https://git.reviewboard.kde.org/r/116767/diff/ Testing --- Configures, builds and installs. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116767: Clean up CMake code from pre-splitting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/#review52829 --- This review has been submitted with commit 62ff6e79ac46d2e2e2878081745cd37f53b5857d by Alex Merry to branch master. - Commit Hook On March 12, 2014, 2:57 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116767/ --- (Updated March 12, 2014, 2:57 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- Clean up CMake code from pre-splitting Remove code that was for building as part of kdelibs, add feature_summary and respect the option to disable tests. Diffs - CMakeLists.txt 4d08bb97a569488849504e04078a71995a1fc53b Diff: https://git.reviewboard.kde.org/r/116767/diff/ Testing --- Configures, builds and installs. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116768: Make our css more future-proof
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116768/#review52830 --- Ship it! Ship It! - Alex Merry On March 12, 2014, 3:49 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116768/ --- (Updated March 12, 2014, 3:49 p.m.) Review request for KDE Frameworks and Aurélien Gâteau. Repository: kapidox Description --- - Remove outdated files - Extend the doxygen.css file provided by Doxygen instead of replacing it - Trim kde.css to contain only necessary lines Diffs - src/kapidox/data/htmlresource/print.css 16818ba src/kapidox/data/htmlresource/tabs.css a61552a src/kapidox/data/htmlresource/kde.css c6c2d86 src/kapidox/data/header.html 109045e src/kapidox/data/htmlresource/flat.css e1db552 src/kapidox/__init__.py 3127357 src/kapidox/data/doxygen.css a0f924e Diff: https://git.reviewboard.kde.org/r/116768/diff/ Testing --- Regenerated doc for all frameworks using kgenframeworksapidox. Browsed around. Did not spot regressions. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116760: Remove kcookiescfg update scripts
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116760/#review52831 --- well I'm still hoping some sort of migration of the useful config data will happen - David Faure On March 12, 2014, 3:44 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116760/ --- (Updated March 12, 2014, 3:44 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- Am I right about this? Remove kcookiescfg update scripts kf5 stores configuration in different locations to kde4, so there should be nothing to update. Diffs - src/ioslaves/http/kcookiejar/CMakeLists.txt 2bb4cc232b7011d38033da10bb312a12215cb29d src/ioslaves/http/kcookiejar/kcookiescfg.pl 7fef01d5c208a00987b3594411ce06799457 src/ioslaves/http/kcookiejar/kcookiescfg.upd 745f545090875d5d24e1e6bb397af9926704350f Diff: https://git.reviewboard.kde.org/r/116760/diff/ Testing --- Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115918: Fix kservice_desktop_to_json for Visual Studio
On March 4, 2014, 8:45 p.m., Kevin Ottens wrote: And I agree with Aurélien, a bug should be filed and Stephen involved in that issue. Stephen Kelly wrote: Please provide a minimal testcase. The feature is unit tested in cmake. If it's broken, it needs to be fixed soon (before the final 3.0 release). Alexander Richardson wrote: I'll try to reproduce it with a minimal test case, however I am away until Sunday, so it will be a while. Stephen Kelly wrote: I've pushed a fix to the cmake next branch. Please test it instead of committing this patch. Another point of note is that you shouldn't read the LOCATION property from the target. You get a policy warning about that. Use the target name directly instead. Alexander Richardson wrote: I can confirm that using cmake next branch fixes the issue. How do I check that the currently used CMake version contains that fix? Kevin Ottens wrote: No idea, anyone? Maybe you want 'NOT VERSION_LESS 3.0'. - Stephen --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115918/#review51946 --- On March 4, 2014, 8:45 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115918/ --- (Updated March 4, 2014, 8:45 p.m.) Review request for KDE Frameworks and Stephen Kelly. Repository: kservice Description --- Fix kservice_desktop_to_json for Visual Studio The CMake generator for Visual Studio cannot handle the new build-time kservice_desktop_to_json macro - fall back to configure-time generation Diffs - KF5ServiceMacros.cmake f70a185f4cd48293cb9f1e2ca4cf13defaf2aec3 Diff: https://git.reviewboard.kde.org/r/115918/diff/ Testing --- ktexteditor fails to build before this patch (because moc can't find the .json file), with the patch it compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116670: Use absolute paths to refer to entities/elements defined in kdoctools
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116670/#review52838 --- This review has been submitted with commit 4190d2e5dff43ef91ce40f4d8be6d834024c0df3 by Luigi Toscano to branch master. - Commit Hook On March 9, 2014, 2:39 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116670/ --- (Updated March 9, 2014, 2:39 p.m.) Review request for KDE Frameworks, kdewin, Alexander Richardson, and Harald Fernengel. Repository: kde4support Description --- The processing of documentation still using DocBookXML 4.2, which depends on kde4support, currently fails on our Jenkins instance where each module has a different installation prefix with an error like: --- Generating kate.1 file:///usr/share/xml/docbook/schema/dtd/4.2/dbpoolx.mod:215: warning: failed to load external entity file:///srv/jenkins/install/linux/x86_64/g++/kf5- qt5/frameworks/kde4support/inst/share/kdoctools5/customization/dtd/rdbpool.elements %rdbpool; ^ Entity: line 1: %rdbpool; ^ --- and similar for the other element/entity files referred. This does not happen in the normal installation because in that case those files will be found in the same directory ($prefix/share/kdoctools5/customization/dtd) and the relative path will work. The attached patch change the paths in the compatibiliy DTD to be absolute path instead. I have to question about this solution (kdewin group): will it work on windows? I guess so, as the directory returned by KDOCTOOLS_CUSTOMIZATION_DIR is set by KF5DocToolsConfig.cmake using KDOCTOOLS_DATA_INSTALL_DIR which uses PACKAGE_PREFIX_DIR, but I'm not totally sure. Similar question for MacOSX. Diffs - src/customization/dtd/kdex.dtd.cmake 1f75dd9 src/CMakeLists.txt d436846 Diff: https://git.reviewboard.kde.org/r/116670/diff/ Testing --- It compiles, and a module whose documentation still uses the old DTD (oktate) compiles correctly. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116037: change entities to reflect new branding
On March 4, 2014, 8:48 p.m., Kevin Ottens wrote: Ship It! Kevin Ottens wrote: Any news? Anything blocking this patch from being committed? Well I still need to fix the i18n issues and change up the entities a little bit to reflect the feedback to https://mail.kde.org/pipermail/kde-frameworks-devel/2014-February/012182.html Plus I've been a bit too busy to even reply to list mail. :-( Will try and finish this up by the end of the week. - T.C. --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116037/#review51948 --- On Feb. 25, 2014, 1:02 a.m., T.C. Hollingsworth wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116037/ --- (Updated Feb. 25, 2014, 1:02 a.m.) Review request for Documentation, KDE Frameworks, Luigi Toscano, Burkhard Lück, and Yuri Chornoivan. Repository: kdoctools Description --- -kde; is no longer semantically identified as an acronym, but instead as an organization -new translatable entities for KDE Frameworks are added -new translatable entities for the KDE SC are added -new translatable entities for the various incarnations of Plasma are added with this I can finally kill the temporary entities that have been in kde-runtime.git/doc/fundamentals for over a year now. :-) Diffs - src/customization/en/user.entities 47bfe0d src/customization/entities/general.entities 183fbc9 Diff: https://git.reviewboard.kde.org/r/116037/diff/ Testing --- kdoctools still builds, kate docs still meinproc5 successfully Thanks, T.C. Hollingsworth ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: src/kcompletionbox.h, line 228 https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228 I wouldn't leave the implementation here. Move it to the .cpp file, this way it can be changed in the future, if it's required for some reason. Also there's a typo in the method name. David Gil Oliva wrote: Alex Merry inlined deprecated methods in https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way to go... Alex Merry wrote: Well, there's a balance to be struck: putting them in the header ensures there is no runtime cost to programs that don't use the deprecated methods, as the code should be optimised away, that the library is always binary-compatible even if you compile it with deprecated code disabled (*_NO_DEPRECATED) and makes the header code document how to replace existing calls. The downsides are an inability to fix the code later and an inability to access members of a private d-pointer class. Neither of those are an issue here, as we're just renaming the method. tl;dr: I disagree with Aleix, and think it should stay in the header. Oh, and Aleix: could you please select the whole method when you're doing a comment like this, rather than just the first line? Otherwise it's a pain to see what you're referring to. Thanks :-) Aleix Pol Gonzalez wrote: Well, I wouldn't bother about runtime penalty given that it's deprecated and we shouldn't be using it anyway. Also we can't make assumptions on how things are going to be optimized. But it's ok, I don't think it's worth discussing further, I doubt this is going to be a problem in the future. Alex Merry wrote: I mean the runtime penalty for things that *don't* use it. If it's header-only, it doesn't go in the library, so there is no load-time symbol-lookup penalty, and no code-size penalty. Admittedly, both of those are probably negligible... Thank you both of you for the useful hints. With this little discussion I have learned some criteria to decide what to do now and in the future. On March 11, 2014, 11:17 p.m., David Gil Oliva wrote: Have you looked through the uses of the un-slotted methods? (lxr.kde.org). Maybe there's a reason for that... :/ David Gil Oliva wrote: Maybe I'm totally wrong, but I can't imagine any way that a getter can be useful as a slot. connect (widget, SLOT(valueChanged(), completionBox, SIGNAL(isTabHandling()); Does it make sense?¿?:-/ Alex Merry wrote: Non-void slots are only useful if they work as a slot (ie: have some sort of side-effect) and might be called directly or with QMetaObject::invokeMethod(). If the method is const (like a getter), there's no point having it a slot at all; if you want to be able to use it with invokeMethod, you can just make it Q_INVOKABLE. Aleix Pol Gonzalez wrote: Well, having const methods as slots doesn't make sense indeed, if it's not for exposing on the meta object system, that's why I said you could do a fast lxr. I just did, didn't find anything relevant. Thank you for looking it up :-) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 11, 2014, 10:32 p.m.) Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 12, 2014, 10:54 p.m.) Review request for KDE Frameworks. Changes --- Address Alex and Aleix suggestions. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs (updated) - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 12, 2014, 10:57 p.m.) Review request for KDE Frameworks. Changes --- Fix non-commited change. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() - cancelled (private method) -Deprecate sizeAndPosition() -- resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs (updated) - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116684: Improve API in KCompletionBase
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116684/
---
(Updated March 12, 2014, 11:06 p.m.)
Review request for KDE Frameworks.
Changes
---
Change include to header, following Kévin's advice.
Repository: kcompletion
Description
---
Improve API in KCompletionBase
getKeyBindings - keyBindingMap
getKeyBinding - keyBinding
NOTICE: The following part is commented out because it gives me errors and I
need some help to find out what actually happens:
/**
* @deprecated since 5.0, use keyBindingMap instead
*/
#ifndef KDE_NO_DEPRECATED
KCOMPLETION_DEPRECATED KeyBindingMap getKeyBindings() const
{
return keyBindingMap();
}
#endif
Error:
In file included from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21:0:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:
In member function ‘KCompletionBase::KeyBindingMap
KCompletionBase::getKeyBindings() const’:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5:
error: return type ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’ is incomplete
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30:
error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka
class QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
In file included from
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:23,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21:
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error:
declaration of ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
In file included from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:27:0,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:
In member function ‘KCompletionBase::KeyBindingMap
KCompletionBase::getKeyBindings() const’:
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5:
error: return type ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’ is incomplete
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30:
error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka
class QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
In file included from
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56,
from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:25,
from
/home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23:
/opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error:
declaration of ‘KCompletionBase::KeyBindingMap {aka class
QMapKCompletionBase::KeyBindingType, QListQKeySequence }’
make[2]: *** [src/CMakeFiles/KF5Completion.dir/kcompletionbase.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs
Diffs (updated)
-
src/kcompletionbase.h 1dedd2d
src/kcompletionbase.cpp 1e251d1
src/klineedit.cpp ae15093
Diff: https://git.reviewboard.kde.org/r/116684/diff/
Testing
---
It compiles and (auto)tests pass without the conflicting part (to be
uncommented).
Thanks,
David Gil Oliva
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/#review52856 --- This review has been submitted with commit 83babc41d2ee44473e72466aa07554769375fb51 by Luigi Toscano to branch master. - Commit Hook On March 10, 2014, 5:46 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 10, 2014, 5:46 p.m.) Review request for Documentation, KDE Frameworks and David Faure. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/#review52855 --- This review has been submitted with commit 19411c1e76c7ebe915a7eef844bb537e48110439 by Luigi Toscano to branch master. - Commit Hook On March 10, 2014, 5:46 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 10, 2014, 5:46 p.m.) Review request for Documentation, KDE Frameworks and David Faure. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 13, 2014, 12:48 a.m.) Status -- This change has been marked as submitted. Review request for Documentation, KDE Frameworks and David Faure. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116685: Import documentationnotfound in kio, used by kio-help
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116685/ --- (Updated March 13, 2014, 12:48 a.m.) Status -- This change has been marked as submitted. Review request for Documentation, KDE Frameworks and David Faure. Repository: kio Description --- The special document documentationnotfound is provided by kde-runtime/khelpcenter and if found it is used by kio-help when the requested documentation is not available. I think it makes more sense to have it where kio-help lives. Please note the import would include the (short) history of the documentationnotfound directory (and its index.docbook and CMakeLists.txt) extracted from kde-runtime history. Diffs - docs/kioslave/help/CMakeLists.txt 73e1506 docs/kioslave/help/documentationnotfound/CMakeLists.txt PRE-CREATION docs/kioslave/help/documentationnotfound/index.docbook PRE-CREATION src/ioslaves/help/kio_help.cpp d693442 Diff: https://git.reviewboard.kde.org/r/116685/diff/ Testing --- kioclient cat help:/foobar shows the content of documentationnotfound instead of There is no documentation available for /foobar. Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Build failed in Jenkins: kdelibs_stable #1038
See http://build.kde.org/job/kdelibs_stable/1038/ -- Started by remote host 127.0.0.1 with note: Triggered by commit Building remotely on LinuxSlave - 3 (PACKAGER LINBUILDER) in workspace http://build.kde.org/job/kdelibs_stable/ws/ Running Prebuild steps [kdelibs_stable] $ /bin/sh -xe /tmp/hudson4166813982360905916.sh + /home/jenkins/scripts/setup-env.sh Preparing to perform KDE Continuous Integration build == Setting Up Sources From git://anongit.kde.org/kdelibs 61bbbda..2dbcc83 KDE/4.12 - origin/KDE/4.12 0c133b0..a5e1cc8 KDE/4.13 - origin/KDE/4.13 b036eb3..49a4e20 master - origin/master Branch jenkins set up to track remote branch KDE/4.13 from origin. == Cleaning Source Tree HEAD is now at 0c133b0 Merge remote-tracking branch 'origin/KDE/4.12' into KDE/4.13 Removing build/ Removing install/ Success build forhudson.tasks.Shell@216b907c Fetching changes from the remote Git repository Fetching upstream changes from git://anongit.kde.org/kdelibs ERROR: Couldn't find any revision to build. Verify the repository and branch configuration for this job. [WARNINGS] Skipping publisher since build result is FAILURE Recording test results ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
On March 4, 2014, 7:42 p.m., David Faure wrote: I don't get it. What's the problem with sending a mimetype for empty files? I would think this is actually expected - for all files, including empty ones. Why does this fix the bug? Dawit Alemayehu wrote: It fails and ends up sending an error message. See the if statement in while(true) loop in ftpSendMimeType. Even if that check does not fail and the while(true) loop completes successfully, the next statement if (!buffer.isEmpty()) will be false for empty files. Hence it is completely useless to call ftpSendMimeType from ftpGet for empty files. Dawit Alemayehu wrote: dfaure: any further questions on this? I still think this is wrong. By contract the kioslave must emit a mimetype. Instead, ftpSendMimeType should be fixed, to detect the case where totalSize is 0, and skip the loop in that case since we have nothing to read. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/#review51922 --- On March 2, 2014, 2:20 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 2, 2014, 2:20 p.m.) Review request for kdelibs and David Faure. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs - kioslave/ftp/ftp.cpp 5bb2e8d Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Review Request 116524: Make kio_ftp work with ftp server that don't support absolute path with SIZE command
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116524/#review52715 --- kioslave/ftp/ftp.cpp https://git.reviewboard.kde.org/r/116524/#comment37182 Given that m_currentPath might not end with a '/' (as indicated by the if statement 2 lines below), this is buggy. m_currentPath=/home/adawit/dev path=/home/adawit/development Let's assume both dirs exist. ++pos and path.mid(pos) will lead to lopment. You should make a local slash-terminated copy of m_currentPath before calling startsWith(). - David Faure On March 7, 2014, 6:48 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116524/ --- (Updated March 7, 2014, 6:48 a.m.) Review request for kdelibs and David Faure. Bugs: 326292 http://bugs.kde.org/show_bug.cgi?id=326292 Repository: kdelibs Description --- This patch changes Ftp::ftpSize such that it has support for servers that do not allow absolute paths with the SIZE command. That means when sending the command SIZE /somefile fails, it will try sending SIZE somefile before giving up. See bug report for details. Diffs - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116524/diff/ Testing --- Installed Ftp server from bug report on an Android device and run tests. Thanks, Dawit Alemayehu
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
On March 4, 2014, 7:42 p.m., David Faure wrote: I don't get it. What's the problem with sending a mimetype for empty files? I would think this is actually expected - for all files, including empty ones. Why does this fix the bug? Dawit Alemayehu wrote: It fails and ends up sending an error message. See the if statement in while(true) loop in ftpSendMimeType. Even if that check does not fail and the while(true) loop completes successfully, the next statement if (!buffer.isEmpty()) will be false for empty files. Hence it is completely useless to call ftpSendMimeType from ftpGet for empty files. Dawit Alemayehu wrote: dfaure: any further questions on this? David Faure wrote: I still think this is wrong. By contract the kioslave must emit a mimetype. Instead, ftpSendMimeType should be fixed, to detect the case where totalSize is 0, and skip the loop in that case since we have nothing to read. Ahh... I forgot that the mimeType signal MUST be emitted. But even if we avoid the while loop the if statement also prevents the emission of that signal. What mime-type should be emitted in case of zero sized files would the better question I guess. KMimeType::defaultMimeType()? - Dawit --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/#review51922 --- On March 2, 2014, 2:20 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 2, 2014, 2:20 p.m.) Review request for kdelibs and David Faure. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs - kioslave/ftp/ftp.cpp 5bb2e8d Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
On March 4, 2014, 7:42 p.m., David Faure wrote: I don't get it. What's the problem with sending a mimetype for empty files? I would think this is actually expected - for all files, including empty ones. Why does this fix the bug? Dawit Alemayehu wrote: It fails and ends up sending an error message. See the if statement in while(true) loop in ftpSendMimeType. Even if that check does not fail and the while(true) loop completes successfully, the next statement if (!buffer.isEmpty()) will be false for empty files. Hence it is completely useless to call ftpSendMimeType from ftpGet for empty files. Dawit Alemayehu wrote: dfaure: any further questions on this? David Faure wrote: I still think this is wrong. By contract the kioslave must emit a mimetype. Instead, ftpSendMimeType should be fixed, to detect the case where totalSize is 0, and skip the loop in that case since we have nothing to read. Dawit Alemayehu wrote: Ahh... I forgot that the mimeType signal MUST be emitted. But even if we avoid the while loop the if statement also prevents the emission of that signal. What mime-type should be emitted in case of zero sized files would the better question I guess. KMimeType::defaultMimeType()? No, application/x-zerosize. (defaultMimeType means unknown) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/#review51922 --- On March 2, 2014, 2:20 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 2, 2014, 2:20 p.m.) Review request for kdelibs and David Faure. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs - kioslave/ftp/ftp.cpp 5bb2e8d Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 12, 2014, 11:35 a.m.) Review request for kdelibs and David Faure. Changes --- Updated patch based on feedback. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs (updated) - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Review Request 116524: Make kio_ftp work with ftp server that don't support absolute path with SIZE command
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116524/ --- (Updated March 12, 2014, 11:36 a.m.) Review request for kdelibs and David Faure. Changes --- Updated patch based on feedback. Bugs: 326292 http://bugs.kde.org/show_bug.cgi?id=326292 Repository: kdelibs Description --- This patch changes Ftp::ftpSize such that it has support for servers that do not allow absolute paths with the SIZE command. That means when sending the command SIZE /somefile fails, it will try sending SIZE somefile before giving up. See bug report for details. Diffs (updated) - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116524/diff/ Testing --- Installed Ftp server from bug report on an Android device and run tests. Thanks, Dawit Alemayehu
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 12, 2014, 11:39 a.m.) Review request for kdelibs and David Faure. Changes --- Reverted incorrect patch update. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs (updated) - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 12, 2014, 12:47 p.m.) Review request for kdelibs and David Faure. Changes --- Updated patch to send the proper mimetype for completely empty files (0 bytes). Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs (updated) - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116523/#review52743
---
kioslave/ftp/ftp.cpp
https://git.reviewboard.kde.org/r/116523/#comment37197
A bit hard to read compared to
if (m_size != 0) { while(true) { ... } }
because m_size doesn't change, so it's weird to read while the size is not
0 - it either was, or it will never be.
kioslave/ftp/ftp.cpp
https://git.reviewboard.kde.org/r/116523/#comment37198
the alternative to the if+while is to just if(m_size==0) {
mimeType(zerosize); return; } before the while.
- David Faure
On March 12, 2014, 12:47 p.m., Dawit Alemayehu wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116523/
---
(Updated March 12, 2014, 12:47 p.m.)
Review request for kdelibs and David Faure.
Bugs: 323491
http://bugs.kde.org/show_bug.cgi?id=323491
Repository: kdelibs
Description
---
The attached patch fixes a bug where copying empty files (size == 0) from an
ftp server to any other remote server (sftp, ftp) results in an error message
that states the file could not be opened.
Diffs
-
kioslave/ftp/ftp.cpp ddc6eaf
Diff: https://git.reviewboard.kde.org/r/116523/diff/
Testing
---
Attempt to copy an empty file from any ftp server to a remote destination,
e.g. sftp server.
Thanks,
Dawit Alemayehu
Re: Review Request 116524: Make kio_ftp work with ftp server that don't support absolute path with SIZE command
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116524/#review52744 --- Ship it! Ship It! - David Faure On March 12, 2014, 11:36 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116524/ --- (Updated March 12, 2014, 11:36 a.m.) Review request for kdelibs and David Faure. Bugs: 326292 http://bugs.kde.org/show_bug.cgi?id=326292 Repository: kdelibs Description --- This patch changes Ftp::ftpSize such that it has support for servers that do not allow absolute paths with the SIZE command. That means when sending the command SIZE /somefile fails, it will try sending SIZE somefile before giving up. See bug report for details. Diffs - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116524/diff/ Testing --- Installed Ftp server from bug report on an Android device and run tests. Thanks, Dawit Alemayehu
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 12, 2014, 1:26 p.m.) Review request for kdelibs and David Faure. Changes --- updated patch. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs (updated) - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Is kpovmodeler still valid or should it be removed
On Monday 10 March 2014 12:36:59 Burkhard Lück wrote: extragear/graphics has more candidates for this question: kuickshow: no development since 4 years needs an old no longer available library imlib1 to build no deb packages provided BTW: I made a replacement application for this: http://kde-apps.org/content/show.php?content=143977 -- Best regards/Schöne Grüße Martin A: Because it breaks the logical sequence of discussion Q: Why is top posting bad? () ascii ribbon campaign - against html e-mail /\- against proprietary attachments Geschenkideen, Accessoires, Seifen, Kulinarisches: www.bibibest.at
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/#review52761 --- Ship it! Thanks. I hope we don't get into the case of Unknown Size when the size is actually 0, though. That case would still be buggy, and I'm not sure how we could fix it. - David Faure On March 12, 2014, 1:26 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 12, 2014, 1:26 p.m.) Review request for kdelibs and David Faure. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs - kioslave/ftp/ftp.cpp ddc6eaf Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Is kpovmodeler still valid or should it be removed
El Dimecres, 12 de març de 2014, a les 15:04:01, Martin Koller va escriure: On Monday 10 March 2014 12:36:59 Burkhard Lück wrote: extragear/graphics has more candidates for this question: kuickshow: no development since 4 years needs an old no longer available library imlib1 to build no deb packages provided BTW: I made a replacement application for this: http://kde-apps.org/content/show.php?content=143977 Any reason this doesn't live in kde.org? Cheers, Albert
Re: Review Request 115689: Fix khtml/ecma/xmlhttprequest.cpp to properly handle HEAD requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115689/#review52842 --- This review has been submitted with commit 3d857d2171d4182954a2f4e90a2bc9a862b5c560 by Andrea Iacovitti to branch master. - Commit Hook On Feb. 17, 2014, 5:29 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115689/ --- (Updated Feb. 17, 2014, 5:29 p.m.) Review request for kdelibs and Andrea Iacovitti. Bugs: 331007 http://bugs.kde.org/show_bug.cgi?id=331007 Repository: kdelibs Description --- Fix khtml/ecma/xmlttprequest.cpp such that it correctly handles HEAD requests without a noticable delay, i.e. use KIO::mimeType instead of KIO::get. Otherwise, the request will wait for the content which is not returned when doing a stat operation like HEAD. Diffs - khtml/ecma/xmlhttprequest.cpp b3707e7 kio/kio/job.cpp abb3dfd Diff: https://git.reviewboard.kde.org/r/115689/diff/ Testing --- Tested HEAD redirection with http://greenbytes.de/tech/tc/httpredirects/ Thanks, Dawit Alemayehu
Re: Review Request 115651: Fix HTTP redirection handling (3XX status code)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115651/#review52841 --- This review has been submitted with commit 863f775bacf0a8f7a8ec34f558cc46ceabeacdf8 by Andrea Iacovitti to branch master. - Commit Hook On Feb. 14, 2014, 3:50 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115651/ --- (Updated Feb. 14, 2014, 3:50 p.m.) Review request for kdelibs, Andreas Hartmetz and David Faure. Bugs: 330795 http://bugs.kde.org/show_bug.cgi?id=330795 Repository: kdelibs Description --- The attached patch fixes how we handle HTTP redirection. Currently KIO does not correctly handle a 303 See Other response. Instead of converting the redirection request to a GET operation as specified in the RFC, KIO simply repeats the same operation with the redirect URL. Additionally, KIO does not handle redirection of a delete operation that is handled internally. Diffs - kio/DESIGN.metadata 1351119 kio/kio/accessmanager.cpp 7a806e8 kio/kio/job.cpp 13107c2 kioslave/http/http.cpp b13eed1 Diff: https://git.reviewboard.kde.org/r/115651/diff/ Testing --- Run tests at http://greenbytes.de/tech/tc/httpredirects/t301methods.html http://greenbytes.de/tech/tc/httpredirects/t302methods.html http://greenbytes.de/tech/tc/httpredirects/t303methods.html Thanks, Dawit Alemayehu
Re: Review Request 116048: Remove Content-Type header when redirecting to GET
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116048/#review52847 --- This review has been submitted with commit 907898a49d1e9fac613cccf3d36f2862f030508c by Andrea Iacovitti to branch master. - Commit Hook On Feb. 26, 2014, 5:57 a.m., Andrea Iacovitti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116048/ --- (Updated Feb. 26, 2014, 5:57 a.m.) Review request for kdelibs and Dawit Alemayehu. Repository: kdelibs Description --- This fix is for stable branch KDE/4.12 and it's needed because of latest changes in redirection code handling. Noticed while reviewing https://git.reviewboard.kde.org/r/116017/ (Dawit, you may want to rebase it if this goes in). Diffs - kio/kio/job.cpp 9cf2b57 Diff: https://git.reviewboard.kde.org/r/116048/diff/ Testing --- various Thanks, Andrea Iacovitti
Re: Review Request 115864: kio_http: fixes for 30X response code redirection handling
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115864/#review52846 --- This review has been submitted with commit 16052fed9be0cb7d15c39310fc02b3127f6e2504 by Andrea Iacovitti to branch master. - Commit Hook On Feb. 20, 2014, 11:28 a.m., Andrea Iacovitti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115864/ --- (Updated Feb. 20, 2014, 11:28 a.m.) Review request for kdelibs and Dawit Alemayehu. Repository: kdelibs Description --- Check if method has been overridden via CustomHTTPMethod metaData when redirecting POST to GET in case of 301/302 response code. Handle 308 Permanent Redirect response code. Diffs - kioslave/http/http.h 076ad2b kioslave/http/http.cpp 68b5247 kioslave/http/httpauthentication.cpp 80d7995 Diff: https://git.reviewboard.kde.org/r/115864/diff/ Testing --- Thanks, Andrea Iacovitti
Re: Is kpovmodeler still valid or should it be removed
On Wednesday 12 March 2014 19:44:39 Albert Astals Cid wrote: El Dimecres, 12 de març de 2014, a les 15:04:01, Martin Koller va escriure: On Monday 10 March 2014 12:36:59 Burkhard Lück wrote: extragear/graphics has more candidates for this question: kuickshow: no development since 4 years needs an old no longer available library imlib1 to build no deb packages provided BTW: I made a replacement application for this: http://kde-apps.org/content/show.php?content=143977 Any reason this doesn't live in kde.org? good question. There's no reason (except that I'm still a noob in git usage ...) What are the steps to get the app into playground so that I can then request it to move to kdereview ? Or can I start in kdereview given that the app was already released 2 years ago ? (It's hard to find information on techbase regarding this. Most information is about svn and therefore outdated and I find nothing how to start a new app w.r.t to git repository) -- Best regards/Schöne Grüße Martin A: Because it breaks the logical sequence of discussion Q: Why is top posting bad? () ascii ribbon campaign - against html e-mail /\- against proprietary attachments Geschenkideen, Accessoires, Seifen, Kulinarisches: www.bibibest.at
Re: Review Request 116555: Add support for pam-kwallet in kwalletd
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116555/ --- (Updated March 13, 2014, 12:33 a.m.) Status -- This change has been marked as submitted. Review request for KDE Runtime, Release Team and Valentin Rusu. Repository: kde-runtime Description --- This patch adds support for pam-kwallet (in my scratch right now, to be released soon). This is how the new pam works, and why this patch is needed: In order to open the wallet in a secure way we have to try hard to not send the hash on an insecure manner. This is how we achieve that: -pam_kwallet creates a pipe. -pam_kwallet opens a local socket listening somewhere (/tmp/foo.socket for example). -pam_kwallet forks+execv kwallet, passing via arguments the sockets (pipe and local socket). -pam_kwallet sends the hash via the pipe. -kwalletd gets the hash and waits for the environment. -startkde uses socat to send the environment to kwalletd. -kwalletd setups the environment before any Qt code is executed. -kwalletd resumes execution. With this way of executing kwallet we get: -pam_kwallet knows to who it is sending the hash (its on child). -hash is never revealed on shared memory (dbus), since pipes are private to the apps. -ptrace is usually disabled so only root can see the hash on the app memory -no Qt code is executed without the proper environment (same as startkde) -if kwalletd is executed normally (not from pam_kwallet) then it is business as usual. The patch also comes with integration tests that simulate how kwalletd is executed in the pam module. For the release team, I would love to add this to 4.13, after all it is innocuous if kwalletd is not executed via pam_module. Diffs - kwalletd/CMakeLists.txt e9aef16 kwalletd/autotests/CMakeLists.txt PRE-CREATION kwalletd/autotests/kdewallet.kwl PRE-CREATION kwalletd/autotests/kwalletexecuter.h PRE-CREATION kwalletd/autotests/kwalletexecuter.cpp PRE-CREATION kwalletd/autotests/qtest_kwallet.h PRE-CREATION kwalletd/autotests/testpamopen.cpp PRE-CREATION kwalletd/autotests/testpamopennofile.cpp PRE-CREATION kwalletd/main.cpp f9af414 Diff: https://git.reviewboard.kde.org/r/116555/diff/ Testing --- Thanks, Àlex Fiestas
Re: Review Request 116555: Add support for pam-kwallet in kwalletd
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116555/#review52854 --- This review has been submitted with commit f2fe3e75b4ba12d0f99aa09327059a1865891b14 by Àlex Fiestas to branch KDE/4.13. - Commit Hook On March 2, 2014, 11:33 p.m., Àlex Fiestas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116555/ --- (Updated March 2, 2014, 11:33 p.m.) Review request for KDE Runtime, Release Team and Valentin Rusu. Repository: kde-runtime Description --- This patch adds support for pam-kwallet (in my scratch right now, to be released soon). This is how the new pam works, and why this patch is needed: In order to open the wallet in a secure way we have to try hard to not send the hash on an insecure manner. This is how we achieve that: -pam_kwallet creates a pipe. -pam_kwallet opens a local socket listening somewhere (/tmp/foo.socket for example). -pam_kwallet forks+execv kwallet, passing via arguments the sockets (pipe and local socket). -pam_kwallet sends the hash via the pipe. -kwalletd gets the hash and waits for the environment. -startkde uses socat to send the environment to kwalletd. -kwalletd setups the environment before any Qt code is executed. -kwalletd resumes execution. With this way of executing kwallet we get: -pam_kwallet knows to who it is sending the hash (its on child). -hash is never revealed on shared memory (dbus), since pipes are private to the apps. -ptrace is usually disabled so only root can see the hash on the app memory -no Qt code is executed without the proper environment (same as startkde) -if kwalletd is executed normally (not from pam_kwallet) then it is business as usual. The patch also comes with integration tests that simulate how kwalletd is executed in the pam module. For the release team, I would love to add this to 4.13, after all it is innocuous if kwalletd is not executed via pam_module. Diffs - kwalletd/CMakeLists.txt e9aef16 kwalletd/autotests/CMakeLists.txt PRE-CREATION kwalletd/autotests/kdewallet.kwl PRE-CREATION kwalletd/autotests/kwalletexecuter.h PRE-CREATION kwalletd/autotests/kwalletexecuter.cpp PRE-CREATION kwalletd/autotests/qtest_kwallet.h PRE-CREATION kwalletd/autotests/testpamopen.cpp PRE-CREATION kwalletd/autotests/testpamopennofile.cpp PRE-CREATION kwalletd/main.cpp f9af414 Diff: https://git.reviewboard.kde.org/r/116555/diff/ Testing --- Thanks, Àlex Fiestas
