Review Request 111402: make systemsettings use KDE_VERSION_STRING as its version string instead of the never changing 1.0
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111402/ --- Review request for kde-workspace and Ben Cooksley. Description --- The aim is that in the future we can disable outdated versions in the systemsettings product so that bugs.kde.org will reject crash reports from outdated versions. I know systemsettings itself hasn't really changed much between releases. But since systemsettings is the main container of various KCMs (another one is kcmshell4), and the crashes caused by those KCMs are reflected as crashes of systemsettings, it becomes necessary for systemsetings to use a version scheme which can be used to identify different releases. The current never changing '1.0' version doesn't suit that need. Also see my similar requests against plasma-desktop[1] and dolphin[2]. [1] https://git.reviewboard.kde.org/r/109906/ [2] http://lists.kde.org/?l=kfm-develm=137120231924936w=2 Diffs - systemsettings/app/main.cpp 1656179 Diff: http://git.reviewboard.kde.org/r/111402/diff/ Testing --- Thanks, Jekyll Wu
Re: Review Request 110687: DrKonqi should check for disabled version as the very first step in the reporting assistant.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110687/ --- (Updated June 5, 2013, 10:05 a.m.) Review request for KDE Runtime and George Kiagiadakis. Changes --- Fix spelling and grammer errors Description --- As I have said in https://bugs.kde.org/show_bug.cgi?id=315073#c3 , Bugzilla's new and nice behavior (since 4.2.5) of rejecting reports against disabled versions brings a new usability problem to DrKonqi: users spend value time in downloading debug symbols, generating the backtrace, writing all information he/she can recall, but in the end only to find an error dialog telling them (in a not so clear and friendly way) that bugs.kde.org won't accept his/her report. I would propose making version checking the very first step in the reporting assistant: a perfect bug report against an outdated version is still useless. The patch has been created for sometime and works, but unfortunately I don't have much time for coding since then, so it is not as good as what I have planned to make it to be. Nevertheless, I think it is still a good improvement of the current situation. This addresses bug 315073. http://bugs.kde.org/show_bug.cgi?id=315073 Diffs (updated) - drkonqi/CMakeLists.txt 39833d7 drkonqi/drkonqi_globals.h d5f098a drkonqi/productmapping.h f926c9d drkonqi/productmapping.cpp f4e59e5 drkonqi/reportassistantdialog.cpp c296059 drkonqi/reportassistantpages_bugzilla_versioncheck.h PRE-CREATION drkonqi/reportassistantpages_bugzilla_versioncheck.cpp PRE-CREATION drkonqi/reportinterface.h c7e374e drkonqi/reportinterface.cpp 4190c40 drkonqi/ui/assistantpage_versioncheck.ui PRE-CREATION Diff: http://git.reviewboard.kde.org/r/110687/diff/ Testing --- File Attachments rejecting disabled version http://git.reviewboard.kde.org/media/uploaded/files/2013/05/28/drkonqi-version-checking.png reject disabled versions (revised edition) http://git.reviewboard.kde.org/media/uploaded/files/2013/05/30/bugzilla-version-cheking-improved.png Thanks, Jekyll Wu
Re: Review Request 110687: DrKonqi should check for disabled version as the very first step in the reporting assistant.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110687/ --- (Updated May 30, 2013, 7:08 a.m.) Review request for KDE Runtime and George Kiagiadakis. Changes --- Improve wording: use less offensive words. Fix various code style issues Description --- As I have said in https://bugs.kde.org/show_bug.cgi?id=315073#c3 , Bugzilla's new and nice behavior (since 4.2.5) of rejecting reports against disabled versions brings a new usability problem to DrKonqi: users spend value time in downloading debug symbols, generating the backtrace, writing all information he/she can recall, but in the end only to find an error dialog telling them (in a not so clear and friendly way) that bugs.kde.org won't accept his/her report. I would propose making version checking the very first step in the reporting assistant: a perfect bug report against an outdated version is still useless. The patch has been created for sometime and works, but unfortunately I don't have much time for coding since then, so it is not as good as what I have planned to make it to be. Nevertheless, I think it is still a good improvement of the current situation. This addresses bug 315073. http://bugs.kde.org/show_bug.cgi?id=315073 Diffs (updated) - drkonqi/CMakeLists.txt 39833d7 drkonqi/drkonqi_globals.h d5f098a drkonqi/productmapping.h f926c9d drkonqi/productmapping.cpp f4e59e5 drkonqi/reportassistantdialog.cpp c296059 drkonqi/reportassistantpages_bugzilla_versioncheck.h PRE-CREATION drkonqi/reportassistantpages_bugzilla_versioncheck.cpp PRE-CREATION drkonqi/reportinterface.h c7e374e drkonqi/reportinterface.cpp 4190c40 drkonqi/ui/assistantpage_versioncheck.ui PRE-CREATION Diff: http://git.reviewboard.kde.org/r/110687/diff/ Testing --- File Attachments (updated) rejecting disabled version http://git.reviewboard.kde.org/media/uploaded/files/2013/05/28/drkonqi-version-checking.png reject disabled versions (revised edition) http://git.reviewboard.kde.org/media/uploaded/files/2013/05/30/bugzilla-version-cheking-improved.png Thanks, Jekyll Wu
Review Request 110687: DrKonqi should check for disabled version as the very first step in the reporting assistant.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110687/ --- Review request for KDE Runtime and George Kiagiadakis. Description --- As I have said in https://bugs.kde.org/show_bug.cgi?id=315073#c3, Bugzilla's new and nice behavior (since 4.2.5) of rejecting reports against disabled versions brings a new usability problem to DrKonqi: users spend value time in downloading debug symbols, generating the backtrace, writing all information he/she can recall, but in the end only to find an error dialog which tells them (in a not so clear and friendly way) that bugs.kde.org won't receive his/her report. I would propose making version checking the very first step in the reporting assistant: a perfect bug report against a outdated version is still useless. This addresses bug 315073. http://bugs.kde.org/show_bug.cgi?id=315073 Diffs - Diff: http://git.reviewboard.kde.org/r/110687/diff/ Testing --- Thanks, Jekyll Wu
Re: Review Request 110687: DrKonqi should check for disabled version as the very first step in the reporting assistant.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110687/ --- (Updated May 28, 2013, 11:01 a.m.) Review request for KDE Runtime and George Kiagiadakis. Changes --- upload the patch and screenshot Description (updated) --- As I have said in https://bugs.kde.org/show_bug.cgi?id=315073#c3 , Bugzilla's new and nice behavior (since 4.2.5) of rejecting reports against disabled versions brings a new usability problem to DrKonqi: users spend value time in downloading debug symbols, generating the backtrace, writing all information he/she can recall, but in the end only to find an error dialog telling them (in a not so clear and friendly way) that bugs.kde.org won't accept his/her report. I would propose making version checking the very first step in the reporting assistant: a perfect bug report against an outdated version is still useless. The patch has been created for sometime and works, but unfortunately I don't have much time for coding since then, so it is not as good as what I have planned to make it to be. Nevertheless, I think it is still a good improvement of the current situation. This addresses bug 315073. http://bugs.kde.org/show_bug.cgi?id=315073 Diffs (updated) - drkonqi/CMakeLists.txt 39833d7 drkonqi/drkonqi_globals.h d5f098a drkonqi/productmapping.h f926c9d drkonqi/productmapping.cpp f4e59e5 drkonqi/reportassistantdialog.cpp c296059 drkonqi/reportassistantpages_bugzilla_versioncheck.h PRE-CREATION drkonqi/reportassistantpages_bugzilla_versioncheck.cpp PRE-CREATION drkonqi/reportinterface.h c7e374e drkonqi/reportinterface.cpp 4190c40 drkonqi/ui/assistantpage_versioncheck.ui PRE-CREATION Diff: http://git.reviewboard.kde.org/r/110687/diff/ Testing --- File Attachments (updated) rejecting disabled version http://git.reviewboard.kde.org/media/uploaded/files/2013/05/28/drkonqi-version-checking.png Thanks, Jekyll Wu
Re: Review Request 110633: Don't report crashes if version is disabled in Bugzilla
On May 24, 2013, 9:44 p.m., Jekyll Wu wrote: Bugzilla itself (since 4.2.5) already rejects any attempt against disabled versions. So even without this patch, DrKonqi users won't be able to create crash report against disabled versions in the end. From developers POV, you don't need to worry about that. The problem is usability for users. I'm not sure this reused error dialog is more informative than the existing one in https://bugs.kde.org/attachment.cgi?id=78600action=edit. So I'm against this patch in its current simple form. As said in [1][2], I'm working on a patch for the usability improvement and plan to make it into 4.11. I will create a review request today or tomorrow. [1] https://bugs.kde.org/show_bug.cgi?id=315073#c3 [2] https://bugs.kde.org/show_bug.cgi?id=318769#c1 Martin Gräßlin wrote: Bugzilla itself (since 4.2.5) already rejects any attempt against disabled versions. Bugzilla does blcok, but DrKonqi still reports them as unknown version. I know it because I see the crash reports coming in Martin Gräßlin wrote: So I'm against this patch in its current simple form. Are you also against into backporting it to prevent that we stop getting useless crash reports? I worked on this for fixing a real world problem. Just look at: https://bugs.kde.org/buglist.cgi?list_id=661927bug_severity=crashchfieldto=Nowquery_format=advancedchfield=[Bug%20creation]chfieldfrom=2013-01-01version=unspecifiedlongdesc=kwin%20%284.8product=kwinlongdesc_type=allwordssubstr All crashes reported this year against KWin 4.8. Yes the dialog might not be best, but I cannot change it because it's in string freeze. I agree that for 4.11 something better should be done, but this is more for 4.10 and older (especially older). Ben Cooksley wrote: Perhaps it might be worth requesting a string freeze exemption here? Martin Gräßlin wrote: Perhaps it might be worth requesting a string freeze exemption here? We would need that from each and every distribution. It doesn't help that KDE did the freeze exception and Debian is then not accepting the patch because it violates their policy. Those reports are all from KDE SC 4.8.5, where kwin version (using KDE_VERSION_STRING) was in the strange form of 4.8.5 (4.8.5) . For some strange reason I'm still not aware of, bugs.kde.org fails to reject reports with that kind of version (probably due the space in the version) as it should have. I'm well aware of those reports which shouldn't have been accepted at the first place (since I'm subscribed to kde-bugs-dist@), but I haven't got the time to do a good investigation. Anyway, those escaped crash reports are definitely the race cases. I understand you are annoyed by those kwin crash reports from 4.8.5, but backporting this simple patch won't help you much: the DrKonqi from KDE SC 4.8.x simply don't contain the code of fetching version info from bugs.kde.org through the Product.get webservice API, so backing this simple patch to 4.8.x is useless. I added those code not long time ago, as the necessary base of my final goal of preventing outdated crash reports. So if your goal is not seeing those kwin 4.8.5 crash reports anymore, there are two quick solutions: 1. apply this simple drkonqi patch to 4.10.x, then ask distribution packagers(I'm think of Debian Stable) to ship DrKonqi from KDE SC 4.10.4 together with other components from KDE SC 4.8.4/5. I'm not sure how distribution packagers will think of this kind of mixture. 2. prepare a one-line patch to make kwin from KDE 4.8.5 use version 4.8.5 explicitly, instead of using KDE_VERSION_STRING. That way, bugs.kde.org itself will reject those crash reports as it has done for 4.9.{0,1,2,3} . Then ask distribution packers to apply this kwin one-line patch. I think this is more acceptable for distribution packagers. Or maybe that one line patch should be created against kdelibs, changing KDE_VERSION_STRING to the plain 4.8.5 ? - Jekyll --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110633/#review33110 --- On May 24, 2013, 2:54 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110633/ --- (Updated May 24, 2013, 2:54 p.m.) Review request for KDE Runtime, Plasma, Ben Cooksley, and Myriam Schweingruber. Description --- Bugzilla provides a feature to disable versions. This means that the developers do not want to have further reports for this version. Any crash report is by that not helpful any more. So let's just disable reporting crashes
Re: Review Request 110633: Don't report crashes if version is disabled in Bugzilla
On May 24, 2013, 9:44 p.m., Jekyll Wu wrote: Bugzilla itself (since 4.2.5) already rejects any attempt against disabled versions. So even without this patch, DrKonqi users won't be able to create crash report against disabled versions in the end. From developers POV, you don't need to worry about that. The problem is usability for users. I'm not sure this reused error dialog is more informative than the existing one in https://bugs.kde.org/attachment.cgi?id=78600action=edit. So I'm against this patch in its current simple form. As said in [1][2], I'm working on a patch for the usability improvement and plan to make it into 4.11. I will create a review request today or tomorrow. [1] https://bugs.kde.org/show_bug.cgi?id=315073#c3 [2] https://bugs.kde.org/show_bug.cgi?id=318769#c1 Martin Gräßlin wrote: Bugzilla itself (since 4.2.5) already rejects any attempt against disabled versions. Bugzilla does blcok, but DrKonqi still reports them as unknown version. I know it because I see the crash reports coming in Martin Gräßlin wrote: So I'm against this patch in its current simple form. Are you also against into backporting it to prevent that we stop getting useless crash reports? I worked on this for fixing a real world problem. Just look at: https://bugs.kde.org/buglist.cgi?list_id=661927bug_severity=crashchfieldto=Nowquery_format=advancedchfield=[Bug%20creation]chfieldfrom=2013-01-01version=unspecifiedlongdesc=kwin%20%284.8product=kwinlongdesc_type=allwordssubstr All crashes reported this year against KWin 4.8. Yes the dialog might not be best, but I cannot change it because it's in string freeze. I agree that for 4.11 something better should be done, but this is more for 4.10 and older (especially older). Ben Cooksley wrote: Perhaps it might be worth requesting a string freeze exemption here? Martin Gräßlin wrote: Perhaps it might be worth requesting a string freeze exemption here? We would need that from each and every distribution. It doesn't help that KDE did the freeze exception and Debian is then not accepting the patch because it violates their policy. Jekyll Wu wrote: Those reports are all from KDE SC 4.8.5, where kwin version (using KDE_VERSION_STRING) was in the strange form of 4.8.5 (4.8.5) . For some strange reason I'm still not aware of, bugs.kde.org fails to reject reports with that kind of version (probably due the space in the version) as it should have. I'm well aware of those reports which shouldn't have been accepted at the first place (since I'm subscribed to kde-bugs-dist@), but I haven't got the time to do a good investigation. Anyway, those escaped crash reports are definitely the race cases. I understand you are annoyed by those kwin crash reports from 4.8.5, but backporting this simple patch won't help you much: the DrKonqi from KDE SC 4.8.x simply don't contain the code of fetching version info from bugs.kde.org through the Product.get webservice API, so backing this simple patch to 4.8.x is useless. I added those code not long time ago, as the necessary base of my final goal of preventing outdated crash reports. So if your goal is not seeing those kwin 4.8.5 crash reports anymore, there are two quick solutions: 1. apply this simple drkonqi patch to 4.10.x, then ask distribution packagers(I'm think of Debian Stable) to ship DrKonqi from KDE SC 4.10.4 together with other components from KDE SC 4.8.4/5. I'm not sure how distribution packagers will think of this kind of mixture. 2. prepare a one-line patch to make kwin from KDE 4.8.5 use version 4.8.5 explicitly, instead of using KDE_VERSION_STRING. That way, bugs.kde.org itself will reject those crash reports as it has done for 4.9.{0,1,2,3} . Then ask distribution packers to apply this kwin one-line patch. I think this is more acceptable for distribution packagers. Or maybe that one line patch should be created against kdelibs, changing KDE_VERSION_STRING to the plain 4.8.5 ? Martin Gräßlin wrote: Or maybe that one line patch should be created against kdelibs, changing KDE_VERSION_STRING to the plain 4.8.5 ? That's the place where it needs to be done, I think. KWin doesn't set a version number it uses KDE_VERSION_STRING. On the other hand fixing in kdelibs won't fix the problem as it would require to recompile everything. This is btw. not just a problem for KWin (that's just what I am familiar with), it applies to all software from KDE SC. The problem of 4.8.4 and 4.8.5 is common in the complete workspaces and it's not going to go away as it's the version used in Debian Stable and Ubuntu LTS. Would creating versions 4.8.4 (4.8.4) and 4.8.5 (4.8.5) in bugzilla fix the problem? Jekyll Wu wrote: I already created
Re: Review Request 110633: Don't report crashes if version is disabled in Bugzilla
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110633/#review33110 --- Bugzilla itself (since 4.2.5) already rejects any attempt against disabled versions. So even without this patch, DrKonqi users won't be able to create crash report against disabled versions in the end. From developers POV, you don't need to worry about that. The problem is usability for users. I'm not sure this reused error dialog is more informative than the existing one in https://bugs.kde.org/attachment.cgi?id=78600action=edit. So I'm against this patch in its current simple form. As said in [1][2], I'm working on a patch for the usability improvement and plan to make it into 4.11. I will create a review request today or tomorrow. [1] https://bugs.kde.org/show_bug.cgi?id=315073#c3 [2] https://bugs.kde.org/show_bug.cgi?id=318769#c1 - Jekyll Wu On May 24, 2013, 2:54 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110633/ --- (Updated May 24, 2013, 2:54 p.m.) Review request for KDE Runtime, Plasma, Ben Cooksley, and Myriam Schweingruber. Description --- Bugzilla provides a feature to disable versions. This means that the developers do not want to have further reports for this version. Any crash report is by that not helpful any more. So let's just disable reporting crashes for such bugs. If this change gets accepted I intend to backport it to 4.10 and to inform kde-packagers about it to ship it as an update to *all* version they support. This would automatically prevent most duplicates report we get e.g. for KWin. Diffs - drkonqi/reportinterface.cpp 4190c40 Diff: http://git.reviewboard.kde.org/r/110633/diff/ Testing --- See https://bugs.kde.org/show_bug.cgi?id=320217 - the bug was created with version 4.10.60. Afterward the version got disabled and DrKonqi doesn't allow me to report crashes for this version anymore. File Attachments With it disabled http://git.reviewboard.kde.org/media/uploaded/files/2013/05/24/drkonqi-disabled.png Thanks, Martin Gräßlin
Re: Login for bug reporting
On 2013年02月07日 19:02, Jan Kundrát wrote: Are most of these reports coming from DrKonqi? If so, have it fetch the list of supported versions from somewhere and tell the user to upgrade when their version is too old, then. (And don't accidentally prevent the automated reporting when the version is actually an output from `git describe` etc). Cheers, Jan That is not doable until recently. It's on my TODO list for drkonqi and is actually the original motivation for me to start working on drkonqi. Regards Jekyll
Re: Login for bug reporting
On 2013年02月07日 01:56, Kevin Krammer wrote: Hi at FOSDEM I was approached by a person who asked me to relay his dissatisfaction with the requirement of having a KDE Bugzilla account to report crashes via the KDE crash handler dialog. The issue in his case was kind of made worse by having this obstacle appear too late, i.e. after he had followed the instructions to create a useful backtrace and had downloaded several tens of megabytes of debug symbols. Well, there is good reason to not put the account creating/logining operation at the beginning : Not every crash is worth reporting. DrKonqi already does some basic checking before allowing users to report a crash. If the account creating/logining operation is put at the very beginning, then users might be told they should not report this crash AFTER they have spent time in creating the account or logining. I think that would be more annoying than the complaint raised in this thread. They may get the impression of being cheated to waste their time, especially creating one account for nothing. Being a FOSS developer himself he said that he understands the need for having a communication channel with the reported, but just having an email address for that would be sufficient (e.g. Debian's bug tracker works that way). So the question is whether alternative login options [1] are something we could do or whether this is impossible in our setup or just something we don't want to do because of certain drawbacks. As for the questions of supporting alternative authentication, anonymous bug database , etc, I lack the related knowledge to make them happen so I wouldn't make stupid comments. But there is one thing I would like to raise : bugs.kde.org is already receiving TOO MANY crash reports, especially useless reports from outdated versions. We don't lack (useful) crash reports. We lack people/time/method to deal with them. So as long as we still use bugs.kde.org for crash reporting, I'm against any change that makes more (useless) crash reports flooding in. I would rather spend time in the opposite direction. One thing that immediately comes into my mind is preventing crash reports from outdated versions (anything older than 4.9.x at the moment) . That is actually already possible after https://reviewboard.kde.org/r/108512 , and I will try my best to make it happen in 4.11. Regards Jekyll
Re: Login for bug reporting
On 2013年02月07日 17:21, Kevin Krammer wrote: It was definitely the process of creating an account, the developer was explicitly stating that providing an email address isn't the problem. Does the crash report dialog offer the option of creating an account? Does it store the password so that it can be automatically retrieved for further reports or when interacting with the web interface in a browser? It doesn't at the moment. It only offers a link to open https://bugs.kde.org/createaccount.cgi in web browser, which asks users to offer one email address and sends the confirmation mail to that email address. Bugzilla does provide the web service API for creating new account, but in effect it does not make difference. It asks for one email address and sends the confirmation mail , too. It is doable, but I don't see real advantage. My understanding of how bugzilla works is that it sends emails to people registered for a certain bug, so an email address would be sufficient, no? I think bugzilla only cares about email addresses it has already verified (meaning registered accounts). It doesn't care about random email addresses.
Re: Login for bug reporting
On 2013年02月07日 18:29, Martin Gräßlin wrote: Also spend a moment and look at the report. There is multiple times written that we don't want any further comments on the bug and that doesn't help anything. Still attachements, still duplicates. I guess you are talking about https://bugs.kde.org/show_bug.cgi?id=278636. That should be easier to implement after the operation of fetch bug report information is ported to using xml-rpc API. But still, there should first be some way for developers to mark one report as not welcoming DrKonqi anymore. Regards Jekyll
Re: KDEREVIEW: Mangonel
On 2013年01月09日 08:09, Martin Sandsmark wrote: Any reason not to use bugs.kde.org? Fixed. Hi, I see you made the change : -aboutData-setBugAddress(QByteArray(bugs.mango...@tarmack.eu)); +aboutData-setBugAddress(QByteArray(https://bugs.kde.org/;)); Hmm, that is not going to work. Dr.Konqi expects sub...@bugs.kde.org as the bug address of applications using bugs.kde.org. If you plan to use bugs.kde.org as the tracker, then you don't need to call setBugAddress() at all. The default value just works. And don't forget to ask sysadmins to create a mangonel product on bugs.kde.org :) Regards Jekyll
Re: Review Request: Correction of bug 235710 : Plasma Wallpaper Slideshow to periodially recheck contents of image folder
On Jan. 5, 2013, 7:49 a.m., Erwan MATHIEU wrote: Well, what am I supposed to do now ? I don't have a write access to the git, so do I have to wait for someone to include the patch ? You need to first get a Ship It! approval of the patch from some maintainer. Only then should you worry about how to git it committed . - Jekyll --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107821/#review24723 --- On Dec. 21, 2012, 5:11 p.m., Erwan MATHIEU wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107821/ --- (Updated Dec. 21, 2012, 5:11 p.m.) Review request for kde-workspace. Description --- Patch which corrects bug 235710 : Plasma Wallpaper Slideshow to periodially recheck contents of image folder This addresses bug 235710. http://bugs.kde.org/show_bug.cgi?id=235710 Diffs - plasma/generic/wallpapers/image/backgroundlistmodel.h a289f49 plasma/generic/wallpapers/image/backgroundlistmodel.cpp f5a4dce plasma/generic/wallpapers/image/image.h 417f5a7 plasma/generic/wallpapers/image/image.cpp 006a748 Diff: http://git.reviewboard.kde.org/r/107821/diff/ Testing --- Many pictures in many folders with many subfolders, adding and removing files (sometimes no more files available) Thanks, Erwan MATHIEU
Re: Review Request: ksysguard - Add rowspan/colspan support for displays
On Jan. 4, 2013, 8:13 p.m., John Tapsell wrote: Ship It! Well, I will push this patch with the assumption that you are busy and Arnav doesn't have commit access yet. - Jekyll --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107970/#review24696 --- On Dec. 28, 2012, 3:50 a.m., Arnav Singh wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107970/ --- (Updated Dec. 28, 2012, 3:50 a.m.) Review request for kde-workspace. Description --- I've added support for sensor displays in KSysGuard to have row spans and column spans. Apart from adding rowSpan and columnSpan arguments to the method signatures, I've also removed the internal list (WorkSheet::mDisplayList) used to contain all the sensor displays. This list used to be used to derived the row and column of the displays based on their index in the list. Since I now need to maintain rowSpan and columnSpan information as well, I just removed the list entirely and get all my data from mGridLayout. As a result, another change in the method signatures is the replacement of the index parameter with row and column parameters. An extra advantage of doing it this way is that widgets don't shift around when resizing the grid. Another advantage is that blank spaces between the widgets are now possible. Not to mention, not maintaining the layout information outside of the actual layout component (mGridLayout) seems a clearer design to me. This addresses bug 311925. http://bugs.kde.org/show_bug.cgi?id=311925 Diffs - ksysguard/gui/WorkSheet.h 9f4806d ksysguard/gui/WorkSheet.cpp b20f077 Diff: http://git.reviewboard.kde.org/r/107970/diff/ Testing --- Works on 4.9.4 Screenshots --- Example http://git.reviewboard.kde.org/r/107970/s/936/ Thanks, Arnav Singh
Re: Review Request: Make Dr.Konqi remember and showuse the time when the crash happend
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107775/ --- (Updated Jan. 3, 2013, 1:42 p.m.) Review request for KDE Runtime and George Kiagiadakis. Changes --- Add basic information about crash time to help translators. Description --- This patch introduces two changes: 1. show the crash time in the bottom of the General tab 2. use the remembered crash time instead of the current time as part of the suggested file name when saving the backtrace . This addresses bug 309330. http://bugs.kde.org/show_bug.cgi?id=309330 Diffs (updated) - drkonqi/crashedapplication.h 403457d drkonqi/crashedapplication.cpp 67ca58e drkonqi/drkonqi.cpp e1d6222 drkonqi/drkonqibackends.cpp 59f3b35 drkonqi/drkonqidialog.cpp f2e4edf Diff: http://git.reviewboard.kde.org/r/107775/diff/ Testing --- Screenshots --- show crash in General tab http://git.reviewboard.kde.org/r/107775/s/905/ Thanks, Jekyll Wu
Re: Review Request: Do not wait for remaining Dr.Konqi instances without timeout when ending the KDE session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107657/#review23891 --- Ping? If there is no strong objection, I will push the proposed change (timeout adjusted to 15min) tomorrow so that it can catch 4.9.5 - Jekyll Wu On Dec. 10, 2012, 8:16 a.m., Jekyll Wu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107657/ --- (Updated Dec. 10, 2012, 8:16 a.m.) Review request for kde-workspace and George Kiagiadakis. Description --- The existing code in startkde waits for Dr.Konqi instances to finish their jobs (saving or reporting the backtrace). That is good, since it make it possible to report crashes during the logout/shutdown procedure. But waiting for Dr.Konqi without timeout is not good: * When users comes back to their computers hours later, they can hardly remember their working context when trying to logout/shutdown previously. I wouldn't expect those annoyed users are willing or able to create useful report under that situation. * It wastes power ... Some users suggest Dr.Konqi should implement a timeout feature, and one user even provides a patch (I tried). However, I don't think that timeout should be done within Dr.Konqi : * It is not that Dr.Konqi intentionally blocks the procedure. It is the opposite, where startkde itself decides to wait for Dr.Konqi and block itself. So logically, the timeout should be done in startkde, not in Dr.Konqi. * If the timeout is done in Dr.Konqi, then Dr.Konqi should only enable that timeout when the system is in the logout/shutdown procedure. That user provided patch clearly doesn't consider that, and the result is if something (like kded) crashes when my system is idle and I'm away, then I won't even know kded has crashed when I comes back hours later. And detecting whether the system is in the produce of logout/shutdown is tricky. It can check whether org.kde.ksmserver is still reachable, but that only applies to a KDE session. So Dr.Konqi should add extra code to check whether it is running in a KDE session , whether ksmserver has gone, and the timeout itself. That sounds unnecessary extra work. So I propose startkde should add a timeout. The patch uses hardcoded 1 hour ( I think that is already long enough, or maybe still too long) as the timeout. Not sure whether it is worthwhile to make the timeout configurable at compile time or runtime. The only downside is some backtrace might be lost. But I don't think that is a big deal. Crashes during shutdown are rare cases nowadays(I hope I'm right), and users noticing those crashes only hours later are the rare case in rare cases (but very annoying). This addresses bug 126073. http://bugs.kde.org/show_bug.cgi?id=126073 Diffs - startkde.cmake dc6f050 Diff: http://git.reviewboard.kde.org/r/107657/diff/ Testing --- Thanks, Jekyll Wu
Review Request: Make Dr.Konqi remember and showuse the time when the crash happend
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107775/ --- Review request for KDE Runtime and George Kiagiadakis. Description --- This patch introduces two changes: 1. show the crash time in the bottom of the General tab 2. use the remembered crash time instead of the current time as part of the suggested file name when saving the backtrace . This addresses bug 309330. http://bugs.kde.org/show_bug.cgi?id=309330 Diffs - drkonqi/crashedapplication.h 403457d drkonqi/crashedapplication.cpp 67ca58e drkonqi/drkonqi.cpp e1d6222 drkonqi/drkonqibackends.cpp 59f3b35 drkonqi/drkonqidialog.cpp 141679b Diff: http://git.reviewboard.kde.org/r/107775/diff/ Testing --- Screenshots --- show crash in General tab http://git.reviewboard.kde.org/r/107775/s/905/ Thanks, Jekyll Wu
Review Request: Do not wait for remaining Dr.Konqi instances without timeout when ending the KDE session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107657/ --- Review request for kde-workspace and George Kiagiadakis. Description --- The existing code in startkde waits for Dr.Konqi instances to finish their jobs (saving or reporting the backtrace). That is good, since it make it possible to report crashes during the logout/shutdown procedure. But waiting for Dr.Konqi without timeout is not good: * When users comes back to their computers hours later, they can hardly remember their working context when trying to logout/shutdown previously. I wouldn't expect those annoyed users are willing or able to create useful report under that situation. * It wastes power ... Some users suggest Dr.Konqi should implement a timeout feature, and one user even provides a patch (I tried). However, I don't think that timeout should be done within Dr.Konqi : * It is not that Dr.Konqi intentionally blocks the procedure. It is the opposite, where startkde itself decides to wait for Dr.Konqi and block itself. So logically, the timeout should be done in startkde, not in Dr.Konqi. * If the timeout is done in Dr.Konqi, then Dr.Konqi should only enable that timeout when the system is in the logout/shutdown procedure. That user provided patch clearly doesn't consider that, and the result is if something (like kded) crashes when my system is idle and I'm away, then I won't even know kded has crashed when I comes back hours later. And detecting whether the system is in the produce of logout/shutdown is tricky. It can check whether org.kde.ksmserver is still reachable, but that only applies to a KDE session. So Dr.Konqi should add extra code to check whether it is running in a KDE session , whether ksmserver has gone, and the timeout itself. That sounds unnecessary extra work. So I propose startkde should add a timeout. The patch uses hardcoded 1 hour ( I think that is already long enough, or maybe still too long) as the timeout. Not sure whether it is worthwhile to make the timeout configurable at compile time or runtime. The only downside is some backtrace might be lost. But I don't think that is a big deal. Crashes during shutdown are rare cases nowadays(I hope I'm right), and users noticing those crashes only hours later are the rare case in rare cases (but very annoying). This addresses bug 126073. http://bugs.kde.org/show_bug.cgi?id=126073 Diffs - startkde.cmake dc6f050 Diff: http://git.reviewboard.kde.org/r/107657/diff/ Testing --- Thanks, Jekyll Wu
Re: Review Request: Do not wait for remaining Dr.Konqi instances without timeout when ending the KDE session
于 2012年12月10日 19:27, Rolf Eike Beer 写道: The only downside is some backtrace might be lost. But I don't think that is a big deal. Crashes during shutdown are rare cases nowadays(I hope I'm right), and users noticing those crashes only hours later are the rare case in rare cases (but very annoying). I regularly get them, most times something Kontact related. What about just saving the backtrace to disk? Next step could be on next startup to check if there are automatically saved backtraces and ask if to drop/save/report them. Just to make it clear, when I say some backtrace might be lost I mean under the cases that bug 126073 complains: Users think their systems will shutdown as expected but hours later find it hasn't because startkde waits for Dr.Konqi forever. The proposed change will not stand in the way of users who already notice the crash at the first place and want to save/report the backtrace. Saving backtrace to disk is of course good, but unfortunately Dr.Konqi doesn't seem to have a dbus call for that operation at the monent. As for Next step, see the comment from George Kiagiadakis. That is not an easy task and might take a long time to implement or never. On the other hand, bug 126073 is a big annoyance to anyone who has ran across it. And it has been there and annoying users for a long time. Comparing the big benefit and small lost of the proposed change, I think that Some backtraces might be lost is really not a big deal. Regards Jekyll
Re: Review Request: Make sure ksmserver ignores excluded apps when saving session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/ --- (Updated Dec. 9, 2012, 7:50 a.m.) Review request for kdelibs, Plasma and Luboš Luňák. Changes --- update the patch to also work with condition 4) update the description to reflect the patch change Description (updated) --- It is easy to understand why the existing code (usually) fails: * Users are most likely to just specify short names, like dolphin,gwenview,okular,rekonq, instead of /usr/bin/konsole,/usr/bin/gwenview,/usr/bin/okular,/usr/bin/rekonq * When ksmserver saves the session, it usually gets the full names, like /usr/bin/dolphin, unless you have started that dolphin instance by typing dolphin exactly in a shell. So there are four possible combinations : 1). config uses short name, runtime gets short name (this guy starts everything from konsole, never using kio/krun) 2). config uses short name, runtime gets long name (I think this is the most common one) 3). config uses long name, runtime gets short name 4). config uses long name, runtime gets long name (I guess some users use this combination because they find only that way works after trying various workaround...) The existing code works with 1) and 4), the patch now works with 1), 2) and 4) . I don't know whether it make senses to support all combinations . This addresses bug 242760. http://bugs.kde.org/show_bug.cgi?id=242760 Diffs (updated) - ksmserver/server.cpp a65b35a Diff: http://git.reviewboard.kde.org/r/107276/diff/ Testing --- Thanks, Jekyll Wu
Re: Review Request: Make sure ksmserver ignores excluded apps when saving session
On Nov. 11, 2012, 11:13 a.m., Aaron J. Seigo wrote: ksmserver/server.cpp, line 939 http://git.reviewboard.kde.org/r/107276/diff/1/?file=94565#file94565line939 why not check for both program and filename? that should then catch 1, 2 and 4, no? excludeApps could also be pre-processed to include both long paths and filenames which would then allow catching all 4 variations. probably the reason this was written to only catch 1 and 4, however, was in case there were binaries of the same name in different paths that should be treated differently (allowing differentiation by full path).. Lubos will certainly have more insight on this, however. probably the reason this was written to only catch 1 and 4, however, was in case there were binaries of the same name in different paths that should be treated differently (allowing differentiation by full path).. I think that is a valid case, but I also think it is rare. I prefer to first fix this more common problem with a simple patch, and wait to see how many users will notice and complain this rare case not working any more. - Jekyll --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/#review21816 --- On Dec. 9, 2012, 7:50 a.m., Jekyll Wu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/ --- (Updated Dec. 9, 2012, 7:50 a.m.) Review request for kdelibs, Plasma and Luboš Luňák. Description --- It is easy to understand why the existing code (usually) fails: * Users are most likely to just specify short names, like dolphin,gwenview,okular,rekonq, instead of /usr/bin/konsole,/usr/bin/gwenview,/usr/bin/okular,/usr/bin/rekonq * When ksmserver saves the session, it usually gets the full names, like /usr/bin/dolphin, unless you have started that dolphin instance by typing dolphin exactly in a shell. So there are four possible combinations : 1). config uses short name, runtime gets short name (this guy starts everything from konsole, never using kio/krun) 2). config uses short name, runtime gets long name (I think this is the most common one) 3). config uses long name, runtime gets short name 4). config uses long name, runtime gets long name (I guess some users use this combination because they find only that way works after trying various workaround...) The existing code works with 1) and 4), the patch now works with 1), 2) and 4) . I don't know whether it make senses to support all combinations . This addresses bug 242760. http://bugs.kde.org/show_bug.cgi?id=242760 Diffs - ksmserver/server.cpp a65b35a Diff: http://git.reviewboard.kde.org/r/107276/diff/ Testing --- Thanks, Jekyll Wu
Re: [PATCH] Fix buttons accessibility in KMenuEdit (Bug296682)
于 2012年11月22日 02:05, Julien 写道: Hi, This is a patch for the bug https://bugs.kde.org/show_bug.cgi?id=296682 It prevents to make buttons as inactive after having deleted or cutted an item. Thus, this bug does not only concern the delete button after having deleted an item, but also the following buttons : cut and copy, after having deleted OR cutted an item. It's my first patch submit :), I wanted to post it in the ReviewBoard, but it seems that the Kmenuedit group does not exist. Thanks Aha. Just for the record, I posted a similar review request a few weeks ago at https://git.reviewboard.kde.org/r/105778/ . Unfortunately, nobody gave a single comment :( So there are two problems : 1. there is no dedicated maintainer for kmenuedit. Is there? 2. there is no appropriate group for kmenuedit on reviewboard. Plasma devs are neither suitable nor interested in reviewing patches for kmenuedit. As for the patch, no objection from me. Regards Jekyll Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Review Request: Make sure ksmserver ignores excluded apps when saving session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/ --- Review request for kdelibs, Plasma and Luboš Luňák. Description --- It is easy to understand why the existing code (usually) fails: * Users are most likely to just specify short names, like dolphin,gwenview,okular,rekonq, instead of /usr/bin/konsole,/usr/bin/gwenview,/usr/bin/okular,/usr/bin/rekonq * When ksmserver saves the session, it usually gets the full names, like /usr/bin/dolphin, unless you have started that dolphin instance by typing dolphin exactly in a shell. So there are four possible combinations : 1). config uses short name, runtime gets short name (this guy starts everything from konsole, never using kio/krun) 2). config uses short name, runtime gets long name (I think this is the most common one) 3). config uses long name, runtime gets short name 4). config uses long name, runtime gets long name (I guess some users use this combination because they find only that way works after trying various workaround...) The existing code works with 1) and 4), this patch works with 1) and 2) . I don't know whether it make senses to support all combinations This addresses bug 242760. http://bugs.kde.org/show_bug.cgi?id=242760 Diffs - Diff: http://git.reviewboard.kde.org/r/107276/diff/ Testing --- Thanks, Jekyll Wu
Re: Review Request: Make sure ksmserver ignores excluded apps when saving session
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107276/ --- (Updated Nov. 10, 2012, 12:45 p.m.) Review request for kdelibs, Plasma and Luboš Luňák. Changes --- Forgot uploading the patch Description --- It is easy to understand why the existing code (usually) fails: * Users are most likely to just specify short names, like dolphin,gwenview,okular,rekonq, instead of /usr/bin/konsole,/usr/bin/gwenview,/usr/bin/okular,/usr/bin/rekonq * When ksmserver saves the session, it usually gets the full names, like /usr/bin/dolphin, unless you have started that dolphin instance by typing dolphin exactly in a shell. So there are four possible combinations : 1). config uses short name, runtime gets short name (this guy starts everything from konsole, never using kio/krun) 2). config uses short name, runtime gets long name (I think this is the most common one) 3). config uses long name, runtime gets short name 4). config uses long name, runtime gets long name (I guess some users use this combination because they find only that way works after trying various workaround...) The existing code works with 1) and 4), this patch works with 1) and 2) . I don't know whether it make senses to support all combinations This addresses bug 242760. http://bugs.kde.org/show_bug.cgi?id=242760 Diffs (updated) - ksmserver/server.cpp eb3ac18 Diff: http://git.reviewboard.kde.org/r/107276/diff/ Testing --- Thanks, Jekyll Wu
Review Request: Remove the mimetype application/x-konsole
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107167/ --- Review request for kdelibs and David Faure. Description --- See the bug report. Put a simple patch here just in case that bug report gets forgotten. This addresses bug 292378. http://bugs.kde.org/show_bug.cgi?id=292378 Diffs - mimetypes/kde.xml f72eb2f Diff: http://git.reviewboard.kde.org/r/107167/diff/ Testing --- Thanks, Jekyll Wu
Review Request: Add CamelCase wrapper for kcodecaction.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105818/ --- Review request for kdelibs. Description --- There is no CamelCase wrapper for kcodecaction.h. I guess that is an omission. This simple patch adds it. Diffs - includes/CMakeLists.txt 157d321 includes/KCodecAction PRE-CREATION Diff: http://git.reviewboard.kde.org/r/105818/diff/ Testing --- Thanks, Jekyll Wu
Review Request: DrKonqi should report the crashes of kactivitymanagerd to the kactivities product in BKO
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105483/ --- Review request for KDE Runtime, Ivan Čukić and George Kiagiadakis. Description --- There is now the kactivities product in BKO (corresponding to the kactivities repository where kactivitymangerd lives) So I think DrKonqi should forward the crashes of kactivitymangerd to the kactivies product. Diffs - drkonqi/data/mappings 147bcdc Diff: http://git.reviewboard.kde.org/r/105483/diff/ Testing --- Thanks, Jekyll Wu
Review Request: [dolphin] Fix the warning generated by desktop-file-validate for dolphin.desktop
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105288/ --- Review request for KDE Base Apps and Peter Penz. Description --- desktop-file-validate dolphin.dekstop generates this: dolphin.desktop: error: (will be fatal in the future): value FileManager in key Categories in group Desktop Entry requires another category to be present among the following categories: System;FileTools FDO menu specification[1] suggests System;FileTools should be used together with the FileManager category. The patch simply adds the missing FileTools [1] http://standards.freedesktop.org/menu-spec/latest/apa.html Diffs - dolphin/src/dolphin.desktop cd9da17 Diff: http://git.reviewboard.kde.org/r/105288/diff/ Testing --- Thanks, Jekyll Wu
Re: Review Request: update the outdated documention and sample code of kde_terminal_interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105155/ --- (Updated June 7, 2012, 2:44 p.m.) Review request for kdelibs and Konsole. Changes --- Add kdelibs into groups Description (updated) --- main change: 1. update the outdated inline document for kde_terminal_interface(which contained some signals that does not exist ever since KDE 4.0) 2. update the example code to make it really work and avoid using hardcoded library name 'libkonsolepart'. 3. update test/CMakeList.txt to make it stand-alone. I don't think that 'test' subfolder is ever used for building test case. Maybe it is more accurate to rename that folder to 'example'. But I could be wrong. This addresses bug 257350. http://bugs.kde.org/show_bug.cgi?id=257350 Diffs - interfaces/terminal/kde_terminal_interface.h 649d674 interfaces/terminal/test/CMakeLists.txt a0fa93a interfaces/terminal/test/main.h 8a3197a interfaces/terminal/test/main.cc 132cee1 Diff: http://git.reviewboard.kde.org/r/105155/diff/ Testing --- The sample code works fine with kdelibs/4.8 and konsole built from master. Thanks, Jekyll Wu
[Help] Is this wrong usage or a bug of global shortcut?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi all, I am trying to fix bug 259539. The real problem is some global shortcuts of klipper keep resetting to 'None' after it restarts. Here is one problematic line: action-setGlobalShortcut(KShortcut(.), KAction::DefaultShortcut); The intention of that line seems to define default key sequence but does not use it at the very first time. However, the actual result is that global shortcut never respects what the user has customized last time and reset to 'None' after klipper restarts. So my question is : is this the wrong usage of setGlobalShortcut(), or a bug of setGlobalShortcut() ? I guess it is the wrong usage, but I need some confirmation since I'm not familiar with global shortcuts. It is easy to fix. I just want to get some confirmation before doing anything stupid. Knotes has similar problems due to same reason . See bug 239078, 208641, and 268897(They are essentially the same). Jekyll -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.18 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQIcBAEBAgAGBQJO9ockAAoJEOemZ9znWXlAT9IP+wYv00Bxv9sySzYIsnZihUp2 LHSdezX3YJYvOX3qxQwPyE4O34DjEkNFnBz7u0iJc9BFgDsMh2/Sd+imI8kh/vQk Udn6O8CmGT8nAlq1NqNV7CQPyGylfwhfqNer/BNOPRmIIXxqqkphyg8zSKfX2N1J 6hTWCo0VqvOjw87G6lgGvLLIJHitZbQHGc8Fh2lD5F/rDNx4SxD2ePUjoUV5Bne2 lq+TvaJJKquxH37RVNFLJFRxu418sN3Or1PasNxw8KchqWu+pJj8jUHMGF+bPQaz 7p8lmW+uerQ/6MtLylQd90xIQpPTe5OVXuugKaO+zAKXrSnelVsd7KNggZoa3kIu WxBLCrAQZafsNskU59oZBbjWkQ9czf/u1h9w7NiAWFPP1mlzIcVe21s/s2ZhVTFc CXK79HdUMwPro8Tp5ZexRqUPwBTRMR1jb7eBLEzDkwchE+M1kMyKWb6uNO8ULBNZ EzAcLAouSy5o1+pIWYOmUnNfxaLA6RpHEh8mLNIB2r69WPMPWp+lXjaWiCurjSF4 ObrwGXl4r/RDS+t0ihwZod4G4feWiwgDWLTKpwhSaPMFwVjD9BE8m/wqRfvu4JXx 9/2gNI5L678gao0rMgBXFajedWNHfQdStW7K/KyT/AskVn6Pn5jvxIoNwn4DCvyb N9kymWFRxhxCE77MDDUG =veA8 -END PGP SIGNATURE-