Re: Review Request: make folderview compile with Qt 4.7
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104030/#review10787 --- Ship it! Ship It! - David Faure On Feb. 20, 2012, 6:16 p.m., Ralf Jung wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104030/ --- (Updated Feb. 20, 2012, 6:16 p.m.) Review request for KDE Base Apps. Description --- Currently, kde-baseapps does not compile against Qt 4.7, in contrast to what cmake checks. This patch fixes the only place where Qt 4.8 API was used. Diffs - plasma/applets/folderview/actionoverlay.cpp 791cfdf Diff: http://git.reviewboard.kde.org/r/104030/diff/ Testing --- compile-tested Thanks, Ralf Jung
Re: Review Request: New KPart extension for manupilating a browser engine's settings
On Feb. 20, 2012, 7:16 p.m., David Faure wrote: kparts/htmlextension.h, line 245 http://git.reviewboard.kde.org/r/103973/diff/4/?file=50594#file50594line245 KParts::SettingsInterface is a bit generic for a name (it sounds like something that could apply to all parts), maybe this should be HTMLSettingsInterface instead (the qt naming style would be HtmlSettingsInterface, but IMHO we should remain consistent with HTMLExtension). Dawit Alemayehu wrote: The extension is named HtmlExtension so there is no reason not to name the interface HtmlSettingsInterface following the qt naming style as well. Ah OK I got confused by KHTMLSomething, I supose. OK, great. On Feb. 20, 2012, 7:16 p.m., David Faure wrote: kparts/htmlextension.h, line 251 http://git.reviewboard.kde.org/r/103973/diff/4/?file=50594#file50594line251 Browser Attribute seems wrong, this isn't an attribute of the browser (app) but an attribute of the html part. HTMLAttributeType enum, setHTMLAttribute and err... htmlAttribute? The capitalization makes this tricky. Alternatively, SettingPropertyType, settingProperty(), and setSettingProperty(). I changed attribute to property because this looks very much like qobject properties (but attribute is fine with me if you prefer that). Dawit Alemayehu wrote: Ok. I renamed the enums HtmlSettingsType, the getter/setter functions to htmlSettingsProperty and setHtmlSettingsProperty. Is that acceptable ? Perfect. On Feb. 20, 2012, 7:16 p.m., David Faure wrote: kparts/htmlextension.h, line 274 http://git.reviewboard.kde.org/r/103973/diff/4/?file=50594#file50594line274 Should this method return a bool maybe, so that the caller can find out that the part didn't support a specific setting? Dawit Alemayehu wrote: Is it a good idea to return a boolean from setter function call ? You can always call the get function to see if the value was properly set, no ? Or did you want to provide a shortcut ? I always avoid a return in set functions on purpose. Alternatively, we can add bool testHtmlSettingsProperty(...) const;. The idea comes from bool QObject::setProperty(). I think it makes perfect sense. You're asking an object to store a specific setting, and you don't know if it supports it or not. With a bool return value we can catch errors (unsupported setting, or possibly even invalid value). A test method (I guess you mean isHtmlSettingSupported) would only test the first one, and would require more code than a simple bool return value. Getting the value again with a getter sounds too tricky, e.g. your handling of data: url would not make this symmetric. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103973/#review10771 --- On Feb. 20, 2012, 11:22 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103973/ --- (Updated Feb. 20, 2012, 11:22 p.m.) Review request for kdelibs and David Faure. Description --- This patch adds a new KPart extension, BrowserSettingsExtension, for setting as well as accessing a browser engine's properties in a generic fashion from KPart plugins. This is yet another step towards making Konqueror's plugins and settings module independent of the default browser engine in use. IOW, they do not have to directly link to a specific browser engine. Diffs - khtml/khtml_ext.h ced53a3 khtml/khtml_ext.cpp 6e8a846 kparts/htmlextension.h 9833d9a Diff: http://git.reviewboard.kde.org/r/103973/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request: make folderview compile with Qt 4.7
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104030/#review10792 --- This review has been submitted with commit 39baedcf978ac34720ce5b022651abd93d96fb00 by Ralf Jung to branch master. - Commit Hook On Feb. 20, 2012, 6:16 p.m., Ralf Jung wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104030/ --- (Updated Feb. 20, 2012, 6:16 p.m.) Review request for KDE Base Apps. Description --- Currently, kde-baseapps does not compile against Qt 4.7, in contrast to what cmake checks. This patch fixes the only place where Qt 4.8 API was used. Diffs - plasma/applets/folderview/actionoverlay.cpp 791cfdf Diff: http://git.reviewboard.kde.org/r/104030/diff/ Testing --- compile-tested Thanks, Ralf Jung
Re: Review Request: make folderview compile with Qt 4.7
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104030/#review10793 --- This review has been submitted with commit 8de987c45a7fef0f1d36a9f575d5cf9a47b19fd0 by Ralf Jung to branch KDE/4.8. - Commit Hook On Feb. 20, 2012, 6:16 p.m., Ralf Jung wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104030/ --- (Updated Feb. 20, 2012, 6:16 p.m.) Review request for KDE Base Apps. Description --- Currently, kde-baseapps does not compile against Qt 4.7, in contrast to what cmake checks. This patch fixes the only place where Qt 4.8 API was used. Diffs - plasma/applets/folderview/actionoverlay.cpp 791cfdf Diff: http://git.reviewboard.kde.org/r/104030/diff/ Testing --- compile-tested Thanks, Ralf Jung
Re: Review Request: Replicate the existing KConfigDialog constructor and change one argument type
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103716/ --- (Updated Feb. 21, 2012, 3:56 p.m.) Review request for kdelibs and David Faure. Description --- Use case: there are applications like kanagram which would be nice to have running on several platforms, like handsets; for instance Harmattan on N9. It would be nice to use the same settings code generation in certain cases for all the platforms since the additions of KConfigSkeleton on the top of KCoreConfigSkeleton are the font and color settings. These are currently not used in many KDE applications. Hence, it should not be mandatory. The kdeui module is unlikely welcome on mobile platforms, especially in appstores with its sizes and complexity for no real need. KConfigDialogManager has apparently already two constructors; one with KConfigSkeleton argument type, and yet another with KCoreConfigSkeleton. It looks like a situation where the KCoreConfigSkeleton version was added later. KConfigDialog does not have a constructor yet with KCoreConfigSkeleton argument type yet; it has probably somehow been missed so far. Changing the current constructor to KCoreConfigSkeleton usage is not possible in the 4.X major version because of the consequences (ABI breakage). Thereby, the freshly replacated constructor. The proper fix can be filed against frameworks where there is only one, and properly working constructor. Diffs (updated) - kdeui/dialogs/kconfigdialog.h 2ac0eda kdeui/dialogs/kconfigdialog.cpp e815e54 Diff: http://git.reviewboard.kde.org/r/103716/diff/ Testing --- On Archlinux (build test only) Thanks, Laszlo Papp
Re: Review Request: Replicate the existing KConfigDialog constructor and change one argument type
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103716/#review10795 --- Ship it! kdeui/dialogs/kconfigdialog.h http://git.reviewboard.kde.org/r/103716/#comment8756 missing @since 4.8.1 - David Faure On Feb. 21, 2012, 3:56 p.m., Laszlo Papp wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103716/ --- (Updated Feb. 21, 2012, 3:56 p.m.) Review request for kdelibs and David Faure. Description --- Use case: there are applications like kanagram which would be nice to have running on several platforms, like handsets; for instance Harmattan on N9. It would be nice to use the same settings code generation in certain cases for all the platforms since the additions of KConfigSkeleton on the top of KCoreConfigSkeleton are the font and color settings. These are currently not used in many KDE applications. Hence, it should not be mandatory. The kdeui module is unlikely welcome on mobile platforms, especially in appstores with its sizes and complexity for no real need. KConfigDialogManager has apparently already two constructors; one with KConfigSkeleton argument type, and yet another with KCoreConfigSkeleton. It looks like a situation where the KCoreConfigSkeleton version was added later. KConfigDialog does not have a constructor yet with KCoreConfigSkeleton argument type yet; it has probably somehow been missed so far. Changing the current constructor to KCoreConfigSkeleton usage is not possible in the 4.X major version because of the consequences (ABI breakage). Thereby, the freshly replacated constructor. The proper fix can be filed against frameworks where there is only one, and properly working constructor. Diffs - kdeui/dialogs/kconfigdialog.h 2ac0eda kdeui/dialogs/kconfigdialog.cpp e815e54 Diff: http://git.reviewboard.kde.org/r/103716/diff/ Testing --- On Archlinux (build test only) Thanks, Laszlo Papp
Re: Review Request: Replicate the existing KConfigDialog constructor and change one argument type
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103716/#review10796 --- This review has been submitted with commit e249c59263cd3a9bf59c01f3884d6eabc29df3ef by Laszlo Papp to branch KDE/4.8. - Commit Hook On Feb. 21, 2012, 3:56 p.m., Laszlo Papp wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103716/ --- (Updated Feb. 21, 2012, 3:56 p.m.) Review request for kdelibs and David Faure. Description --- Use case: there are applications like kanagram which would be nice to have running on several platforms, like handsets; for instance Harmattan on N9. It would be nice to use the same settings code generation in certain cases for all the platforms since the additions of KConfigSkeleton on the top of KCoreConfigSkeleton are the font and color settings. These are currently not used in many KDE applications. Hence, it should not be mandatory. The kdeui module is unlikely welcome on mobile platforms, especially in appstores with its sizes and complexity for no real need. KConfigDialogManager has apparently already two constructors; one with KConfigSkeleton argument type, and yet another with KCoreConfigSkeleton. It looks like a situation where the KCoreConfigSkeleton version was added later. KConfigDialog does not have a constructor yet with KCoreConfigSkeleton argument type yet; it has probably somehow been missed so far. Changing the current constructor to KCoreConfigSkeleton usage is not possible in the 4.X major version because of the consequences (ABI breakage). Thereby, the freshly replacated constructor. The proper fix can be filed against frameworks where there is only one, and properly working constructor. Diffs - kdeui/dialogs/kconfigdialog.h 2ac0eda kdeui/dialogs/kconfigdialog.cpp e815e54 Diff: http://git.reviewboard.kde.org/r/103716/diff/ Testing --- On Archlinux (build test only) Thanks, Laszlo Papp
Re: Review Request: Make KFileWidget provide default filename even when the protocol doesn't support listing
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103642/#review10805 --- This review has been submitted with commit 37ff4f63f23b36e6025298ec31989475127aac79 by Dawit Alemayehu to branch KDE/4.8. - Commit Hook On Jan. 6, 2012, 9:54 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103642/ --- (Updated Jan. 6, 2012, 9:54 p.m.) Review request for kdelibs and David Faure. Description --- This patch changes the logic KFileWidget::getStartUrl so that a default file name is provided for protocols that do not support listing, e.g. http. This should address the problem reported in bug# 169348 at a central location instead of leaving it up to each application to do the right thing. This addresses bug 169348. http://bugs.kde.org/show_bug.cgi?id=169348 Diffs - kfile/kfilewidget.cpp 960d6c4 Diff: http://git.reviewboard.kde.org/r/103642/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request: New KPart extension for manupilating a browser engine's settings
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103973/#review10807 --- khtml/khtml_ext.cpp http://git.reviewboard.kde.org/r/103973/#comment8760 OK that's more than I expected. A simple return true; would have been enough IMHO, to mean yep, I support this property. This code looks ok, as extra precaution, but is there really a code path in setAutoloadImages which would not just store the given value? My idea was just that in the future when someone adds a new setting, the app code can be notified of whether the setting is supported or not. For now, obviously, khtmlpart implements all the settings, so it would just be return true in every branch of the switch (and return false at the bottom). - David Faure On Feb. 21, 2012, 5:59 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103973/ --- (Updated Feb. 21, 2012, 5:59 p.m.) Review request for kdelibs and David Faure. Description --- This patch adds a new KPart extension, BrowserSettingsExtension, for setting as well as accessing a browser engine's properties in a generic fashion from KPart plugins. This is yet another step towards making Konqueror's plugins and settings module independent of the default browser engine in use. IOW, they do not have to directly link to a specific browser engine. Diffs - khtml/khtml_ext.h ced53a3 khtml/khtml_ext.cpp 6e8a846 kparts/htmlextension.h 9833d9a Diff: http://git.reviewboard.kde.org/r/103973/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request: New KPart extension for manupilating a browser engine's settings
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103973/ --- (Updated Feb. 21, 2012, 6:35 p.m.) Review request for kdelibs and David Faure. Changes --- Return false for only unsupported properties. Description --- This patch adds a new KPart extension, BrowserSettingsExtension, for setting as well as accessing a browser engine's properties in a generic fashion from KPart plugins. This is yet another step towards making Konqueror's plugins and settings module independent of the default browser engine in use. IOW, they do not have to directly link to a specific browser engine. Diffs (updated) - khtml/khtml_ext.h ced53a3 khtml/khtml_ext.cpp 6e8a846 kparts/htmlextension.h 9833d9a Diff: http://git.reviewboard.kde.org/r/103973/diff/ Testing --- Thanks, Dawit Alemayehu
Bugzilla upgrade.
Hi, Our bugzilla instance is an old version. We are preparing to upgrade it to a recent version. This will go in two stages. First we will perform a test upgrade. This will happen on Saturday Februari 25th. We will start at 8PM CET. We expect little inpact, though during the database conversion we expect the current bugzilla to be slower as usual. If this will becomes unworkable, we will close bugs.kde.org during this conversion. If all goes well, we want to do the final conversion on Friday March 2nd. We will start at 8PM CET. During the conversion bugs.kde.org will be unavailable. We expect a downtime of a few hours. We will know more after the test. All work will be coordinated via the #kde-sysadmin. If you want to test the new bugzilla before it goes live (/me waves at the dr.konqi developers) talk to us. Also when you have any other concerns or questions about the upgrade. Best, Tom Albers KDE Sysadmin ps. Any replies which include the word 'Redmine' will be ignored.
Re: Bugzilla upgrade.
On Tuesday 21 February 2012 21:00:33 Tom Albers wrote: Hi, Our bugzilla instance is an old version. We are preparing to upgrade it to a recent version. This will go in two stages. First we will perform a test upgrade. This will happen on Saturday Februari 25th. We will start at 8PM CET. We expect little inpact, though during the database conversion we expect the current bugzilla to be slower as usual. If this will becomes unworkable, we will close bugs.kde.org during this conversion. If all goes well, we want to do the final conversion on Friday March 2nd. We will start at 8PM CET. During the conversion bugs.kde.org will be unavailable. We expect a downtime of a few hours. We will know more after the test. All work will be coordinated via the #kde-sysadmin. If you want to test the new bugzilla before it goes live (/me waves at the dr.konqi developers) talk to us. Also when you have any other concerns or questions about the upgrade. What version will the new bugzilla have? Are the REST/JSON/XMLRPC APIs going to be enabled? Bye and many thanks for giving our bug-pit some love! -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.
Re: Bugzilla upgrade.
On Wed, Feb 22, 2012 at 11:37 AM, Milian Wolff m...@milianw.de wrote: On Tuesday 21 February 2012 21:00:33 Tom Albers wrote: Hi, Our bugzilla instance is an old version. We are preparing to upgrade it to a recent version. This will go in two stages. First we will perform a test upgrade. This will happen on Saturday Februari 25th. We will start at 8PM CET. We expect little inpact, though during the database conversion we expect the current bugzilla to be slower as usual. If this will becomes unworkable, we will close bugs.kde.org during this conversion. If all goes well, we want to do the final conversion on Friday March 2nd. We will start at 8PM CET. During the conversion bugs.kde.org will be unavailable. We expect a downtime of a few hours. We will know more after the test. All work will be coordinated via the #kde-sysadmin. If you want to test the new bugzilla before it goes live (/me waves at the dr.konqi developers) talk to us. Also when you have any other concerns or questions about the upgrade. What version will the new bugzilla have? Are the REST/JSON/XMLRPC APIs going to be enabled? We will likely be upgrading to the latest version of the 4.x series. We have not deliberately disabled REST/JSON/XMLRPC in the past, and will not do so with this update. Bye and many thanks for giving our bug-pit some love! -- Milian Wolff m...@milianw.de http://milianw.de Regards, Ben
Re: Bugzilla upgrade.
On Wednesday 22 February 2012 11:47:36 Ben Cooksley wrote: On Wed, Feb 22, 2012 at 11:37 AM, Milian Wolff m...@milianw.de wrote: On Tuesday 21 February 2012 21:00:33 Tom Albers wrote: Hi, Our bugzilla instance is an old version. We are preparing to upgrade it to a recent version. This will go in two stages. First we will perform a test upgrade. This will happen on Saturday Februari 25th. We will start at 8PM CET. We expect little inpact, though during the database conversion we expect the current bugzilla to be slower as usual. If this will becomes unworkable, we will close bugs.kde.org during this conversion. If all goes well, we want to do the final conversion on Friday March 2nd. We will start at 8PM CET. During the conversion bugs.kde.org will be unavailable. We expect a downtime of a few hours. We will know more after the test. All work will be coordinated via the #kde-sysadmin. If you want to test the new bugzilla before it goes live (/me waves at the dr.konqi developers) talk to us. Also when you have any other concerns or questions about the upgrade. What version will the new bugzilla have? Are the REST/JSON/XMLRPC APIs going to be enabled? We will likely be upgrading to the latest version of the 4.x series. We have not deliberately disabled REST/JSON/XMLRPC in the past, and will not do so with this update. Cool, thanks! bye -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.
Re: Review Request: Write to the correct xmlFile in KToolBar::Private::slotContextShowText()
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103812/#review10817 --- This review has been submitted with commit 576e13d355c34858e8a254a28a100a8b9f7876a8 by Albert Astals Cid to branch KDE/4.8. - Commit Hook On Feb. 20, 2012, 10:33 p.m., Albert Astals Cid wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103812/ --- (Updated Feb. 20, 2012, 10:33 p.m.) Review request for kdelibs and David Faure. Description --- KToolBar::Private::slotContextShowText() was assuming that the xmlgui file it had to write was KGlobal::mainComponent.componentName() + ui.rc; which is obviously wrong since we have a setXMLFile function for a reason. I tried using xmlguiClient-xmlFile() directly but in Okular we use the same the same toolbar name defined in two xml files, so that still did not work because this means we end up with just one KToolbar (yes i know that might be a misuse of the API). So i ended up going through the actioncollections to find the action and get the correct client from there. This addresses bug 292574. http://bugs.kde.org/show_bug.cgi?id=292574 Diffs - kdeui/widgets/ktoolbar.h 69c482e kdeui/widgets/ktoolbar.cpp d17ff39 kdeui/xmlgui/kxmlguibuilder.cpp 6773c31 kdeui/xmlgui/kxmlguifactory_p.cpp 2f81f18 Diff: http://git.reviewboard.kde.org/r/103812/diff/ Testing --- Fixes the issue in Okular, i tested it does still work with Kate that is using the ui.rc scheme. Thanks, Albert Astals Cid
Re: Review Request: Fix KConfigDialogManager fails to handle subclasses of QComboBox with custom property
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103909/ --- (Updated Feb. 21, 2012, 11:54 p.m.) Review request for kdelibs, Ben Cooksley, Eike Hein, Christoph Feck, and Jeremy Paul Whiting. Changes --- Updated with apaku's suggestion to address Christoph's concerns about derived classes without USER property Description --- https://git.reviewboard.kde.org/r/101486/ broke subclasses of QComboBox that have a USER property like KColorCombo, this patch reverts this change and introduces a different code path to ignore the USER property of QComboBox and KComboBox and make it use our custom code. This addresses bug 293702. http://bugs.kde.org/show_bug.cgi?id=293702 Diffs (updated) - kdeui/dialogs/kconfigdialogmanager.cpp 18bc44e kdeui/tests/CMakeLists.txt 63788f6 kdeui/tests/kconfigdialog_unittest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/103909/diff/ Testing --- Ran the attached test, everything worked. Without moving the userproperty = getUserProperty(w); the KColorCombo fails Without adding the s_propertyMap-insert( KComboBox, ); the editable KComboBox fails Thanks, Albert Astals Cid
Re: Review Request: Fix KConfigDialogManager fails to handle subclasses of QComboBox with custom property
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103909/ --- (Updated Feb. 21, 2012, 11:55 p.m.) Review request for kdelibs, Ben Cooksley, Eike Hein, Christoph Feck, and Jeremy Paul Whiting. Description --- https://git.reviewboard.kde.org/r/101486/ broke subclasses of QComboBox that have a USER property like KColorCombo, this patch reverts this change and introduces a different code path to ignore the USER property of QComboBox and KComboBox and make it use our custom code. This addresses bug 293702. http://bugs.kde.org/show_bug.cgi?id=293702 Diffs - kdeui/dialogs/kconfigdialogmanager.cpp 18bc44e kdeui/tests/CMakeLists.txt 63788f6 kdeui/tests/kconfigdialog_unittest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/103909/diff/ Testing (updated) --- Ran the attached test, everything worked. Without moving the userproperty = getUserProperty(w); the KColorCombo fails Without adding the propertyIndex stuff the editable KComboBox fails Thanks, Albert Astals Cid
