Re: Review Request: Support for multiple batteries in battery monitor applet
On Sunday, July 29, 2012 12:45:00 Viranch Mehta wrote: On Sun, Jun 17, 2012 at 2:45 PM, David Edmundson k...@davidedmundson.co.ukwrote: Given this patch is untested (due to Viranch having only one battery) and that we will have a lot of angry users complaining that the feature is missing, can I suggest we submit a version with this patch to kde-look.org, so that it is available in get new stuff. Should the patched applet replace the original applet or be installed with a different plugin name? If its installed as a different applet, it can't be put in the system tray, so what do we want here? We should replace it with the same name, otherwise we'll just have stale applets lying around. And once tested, ship the updated version, maybe in 4.9.1 already? Cheers, -- sebas http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Support for multiple batteries in battery monitor applet
On 02-Aug-2012, at 7:05 PM, Sebastian Kügler wrote: On Sunday, July 29, 2012 12:45:00 Viranch Mehta wrote: On Sun, Jun 17, 2012 at 2:45 PM, David Edmundson k...@davidedmundson.co.ukwrote: Given this patch is untested (due to Viranch having only one battery) and that we will have a lot of angry users complaining that the feature is missing, can I suggest we submit a version with this patch to kde-look.org, so that it is available in get new stuff. Should the patched applet replace the original applet or be installed with a different plugin name? If its installed as a different applet, it can't be put in the system tray, so what do we want here? We should replace it with the same name, otherwise we'll just have stale applets lying around. Then what about the applet that will be shipped in the next release? will it replace this downloaded one again? I think the applets downloaded from get new stuff go into ~/.kde/ which precede the ones that are installed system-wide. so the one downloaded by user might precede the one installed by the next release also. i'm not sure about this though. And once tested, ship the updated version, maybe in 4.9.1 already? the master is open again, i suppose? Thanks, Viranch ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Support for multiple batteries in battery monitor applet
On Sun, Jun 17, 2012 at 2:45 PM, David Edmundson k...@davidedmundson.co.ukwrote: Given this patch is untested (due to Viranch having only one battery) and that we will have a lot of angry users complaining that the feature is missing, can I suggest we submit a version with this patch to kde-look.org, so that it is available in get new stuff. Should the patched applet replace the original applet or be installed with a different plugin name? If its installed as a different applet, it can't be put in the system tray, so what do we want here? Viranch ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Support for multiple batteries in battery monitor applet
On June 16, 2012, 6:40 p.m., Marco Martin wrote: i think the change is on the right path, but should wait for the unfreeze, since is not a trivial fix David Edmundson wrote: Given this patch is untested (due to Viranch having only one battery) and that we will have a lot of angry users complaining that the feature is missing, can I suggest we submit a version with this patch to kde-look.org, so that it is available in get new stuff. This will give us the extra testing before we merge in 4.10, and at least provide a workaround we can share for those users, and the people giving support. Marco Martin wrote: actually a good idea, would give a fixed place to sent people. Marco Martin wrote: uh, this screams syncrotron as well :p We need to get it tested first. Does anybody have a machine with two batteries? I don't, Viranch doesn't, ... The really cool thing is that we can just send a .plasmoid file with the improvements to anyone who runs at least beta1, that should make it possible to find someone who can give feedback. - Sebastian --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/#review14800 --- On June 16, 2012, 3:25 p.m., Viranch Mehta wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/ --- (Updated June 16, 2012, 3:25 p.m.) Review request for Plasma. Description --- This patch implements support for computers with multiple batteries in the battery monitor applet. I'm not sure if I should push it now or after the unfreeze. This review addresses the bug #301533 This addresses bug 301533. http://bugs.kde.org/show_bug.cgi?id=301533 Diffs - plasma/generic/applets/batterymonitor/contents/code/logic.js PRE-CREATION plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml a2ab72a plasma/generic/applets/batterymonitor/contents/ui/batterymonitor.qml 08a46ec Diff: http://git.reviewboard.kde.org/r/105277/diff/ Testing --- Added dummy battery sources in power management engine and tested with it. Works fine, as expected with such sources. Can someone with multiple batteries please test the patch? since I don't have a computer with multiple batteries. Thanks, Viranch Mehta ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Support for multiple batteries in battery monitor applet
On June 16, 2012, 6:40 p.m., Marco Martin wrote: i think the change is on the right path, but should wait for the unfreeze, since is not a trivial fix Given this patch is untested (due to Viranch having only one battery) and that we will have a lot of angry users complaining that the feature is missing, can I suggest we submit a version with this patch to kde-look.org, so that it is available in get new stuff. This will give us the extra testing before we merge in 4.10, and at least provide a workaround we can share for those users, and the people giving support. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/#review14800 --- On June 16, 2012, 3:25 p.m., Viranch Mehta wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/ --- (Updated June 16, 2012, 3:25 p.m.) Review request for Plasma. Description --- This patch implements support for computers with multiple batteries in the battery monitor applet. I'm not sure if I should push it now or after the unfreeze. This review addresses the bug #301533 This addresses bug 301533. http://bugs.kde.org/show_bug.cgi?id=301533 Diffs - plasma/generic/applets/batterymonitor/contents/code/logic.js PRE-CREATION plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml a2ab72a plasma/generic/applets/batterymonitor/contents/ui/batterymonitor.qml 08a46ec Diff: http://git.reviewboard.kde.org/r/105277/diff/ Testing --- Added dummy battery sources in power management engine and tested with it. Works fine, as expected with such sources. Can someone with multiple batteries please test the patch? since I don't have a computer with multiple batteries. Thanks, Viranch Mehta ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Support for multiple batteries in battery monitor applet
On June 16, 2012, 6:40 p.m., Marco Martin wrote: i think the change is on the right path, but should wait for the unfreeze, since is not a trivial fix David Edmundson wrote: Given this patch is untested (due to Viranch having only one battery) and that we will have a lot of angry users complaining that the feature is missing, can I suggest we submit a version with this patch to kde-look.org, so that it is available in get new stuff. This will give us the extra testing before we merge in 4.10, and at least provide a workaround we can share for those users, and the people giving support. actually a good idea, would give a fixed place to sent people. - Marco --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/#review14800 --- On June 16, 2012, 3:25 p.m., Viranch Mehta wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/ --- (Updated June 16, 2012, 3:25 p.m.) Review request for Plasma. Description --- This patch implements support for computers with multiple batteries in the battery monitor applet. I'm not sure if I should push it now or after the unfreeze. This review addresses the bug #301533 This addresses bug 301533. http://bugs.kde.org/show_bug.cgi?id=301533 Diffs - plasma/generic/applets/batterymonitor/contents/code/logic.js PRE-CREATION plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml a2ab72a plasma/generic/applets/batterymonitor/contents/ui/batterymonitor.qml 08a46ec Diff: http://git.reviewboard.kde.org/r/105277/diff/ Testing --- Added dummy battery sources in power management engine and tested with it. Works fine, as expected with such sources. Can someone with multiple batteries please test the patch? since I don't have a computer with multiple batteries. Thanks, Viranch Mehta ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Support for multiple batteries in battery monitor applet
On June 16, 2012, 6:40 p.m., Marco Martin wrote: i think the change is on the right path, but should wait for the unfreeze, since is not a trivial fix David Edmundson wrote: Given this patch is untested (due to Viranch having only one battery) and that we will have a lot of angry users complaining that the feature is missing, can I suggest we submit a version with this patch to kde-look.org, so that it is available in get new stuff. This will give us the extra testing before we merge in 4.10, and at least provide a workaround we can share for those users, and the people giving support. Marco Martin wrote: actually a good idea, would give a fixed place to sent people. uh, this screams syncrotron as well :p - Marco --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/#review14800 --- On June 16, 2012, 3:25 p.m., Viranch Mehta wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/ --- (Updated June 16, 2012, 3:25 p.m.) Review request for Plasma. Description --- This patch implements support for computers with multiple batteries in the battery monitor applet. I'm not sure if I should push it now or after the unfreeze. This review addresses the bug #301533 This addresses bug 301533. http://bugs.kde.org/show_bug.cgi?id=301533 Diffs - plasma/generic/applets/batterymonitor/contents/code/logic.js PRE-CREATION plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml a2ab72a plasma/generic/applets/batterymonitor/contents/ui/batterymonitor.qml 08a46ec Diff: http://git.reviewboard.kde.org/r/105277/diff/ Testing --- Added dummy battery sources in power management engine and tested with it. Works fine, as expected with such sources. Can someone with multiple batteries please test the patch? since I don't have a computer with multiple batteries. Thanks, Viranch Mehta ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Support for multiple batteries in battery monitor applet
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/#review14800 --- i think the change is on the right path, but should wait for the unfreeze, since is not a trivial fix - Marco Martin On June 16, 2012, 3:25 p.m., Viranch Mehta wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105277/ --- (Updated June 16, 2012, 3:25 p.m.) Review request for Plasma. Description --- This patch implements support for computers with multiple batteries in the battery monitor applet. I'm not sure if I should push it now or after the unfreeze. This review addresses the bug #301533 This addresses bug 301533. http://bugs.kde.org/show_bug.cgi?id=301533 Diffs - plasma/generic/applets/batterymonitor/contents/code/logic.js PRE-CREATION plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml a2ab72a plasma/generic/applets/batterymonitor/contents/ui/batterymonitor.qml 08a46ec Diff: http://git.reviewboard.kde.org/r/105277/diff/ Testing --- Added dummy battery sources in power management engine and tested with it. Works fine, as expected with such sources. Can someone with multiple batteries please test the patch? since I don't have a computer with multiple batteries. Thanks, Viranch Mehta ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel