Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
On April 21, 2015, 9:47 p.m., David Edmundson wrote: lookandfeel/contents/components/InfoPane.qml, line 47 https://git.reviewboard.kde.org/r/123459/diff/1/?file=362312#file362312line47 there's a visible here. If your two lines are needed, this isn't. Or this should be changed to the battery.hasBattery instead. Yes, we have to use this visible. My two lines are a bit of overkill. - Antonis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79311 --- On April 24, 2015, 6:22 p.m., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:22 p.m.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:22 p.m.) Review request for Plasma. Changes --- fix typo. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs (updated) - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
On April 22, 2015, 3:46 vorm., Kai Uwe Broulik wrote: -1 that's what the visible: pmSource.data[Battery][Has Cumulative] is for. there just used to be a bug where that property wasn't created in the first place if no battery was present leading to an exception causing the item to remain visible. Antonis Tsiapaliokas wrote: What about checking if the pmSource.data[Battery][Has Cumulative] has been set properly, and if not, then using the pmSource.data[Battery][Has Battery]? I have update my patch with the above change... I don't quite get it. Theoretically Has Cumulative is true when there is a power supply battery and it is false if there isn't. I suppose you're running latest plasma-workspace and powerdevil? I used to have this issue on my desktop but fixed it and it's gone here. Could you paste the output of upower -d for your machine, and also the value of Has Cumulative in plasmaengineexplorer? - Kai Uwe --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79321 --- On April 24, 2015, 6:22 nachm., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:22 nachm.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:20 p.m.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs (updated) - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
On April 22, 2015, 3:46 a.m., Kai Uwe Broulik wrote: -1 that's what the visible: pmSource.data[Battery][Has Cumulative] is for. there just used to be a bug where that property wasn't created in the first place if no battery was present leading to an exception causing the item to remain visible. What about checking if the pmSource.data[Battery][Has Cumulative] has been set properly, and if not, then using the pmSource.data[Battery][Has Battery]? I have update my patch with the above change... - Antonis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79321 --- On April 24, 2015, 6:22 p.m., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:22 p.m.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
On April 22, 2015, 3:46 a.m., Kai Uwe Broulik wrote: -1 that's what the visible: pmSource.data[Battery][Has Cumulative] is for. there just used to be a bug where that property wasn't created in the first place if no battery was present leading to an exception causing the item to remain visible. Antonis Tsiapaliokas wrote: What about checking if the pmSource.data[Battery][Has Cumulative] has been set properly, and if not, then using the pmSource.data[Battery][Has Battery]? I have update my patch with the above change... Kai Uwe Broulik wrote: I don't quite get it. Theoretically Has Cumulative is true when there is a power supply battery and it is false if there isn't. I suppose you're running latest plasma-workspace and powerdevil? I used to have this issue on my desktop but fixed it and it's gone here. Could you paste the output of upower -d for your machine, and also the value of Has Cumulative in plasmaengineexplorer? When i wrote this patch(2 days ago), plasmaengineexplorer didn't had the Has Cumulative. Today i update my setup and plasmaenginexplorer has a value for the Has Cumulative. Yes, i am running the latest plasma-workspace and powerdevil. So since none of us can reproduce the issue anymore, and you have fixed the issue with the Has Cumulative, i guess we can discard this review. Right? - Antonis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79321 --- On April 24, 2015, 6:22 p.m., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:22 p.m.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
On April 22, 2015, 3:46 a.m., Kai Uwe Broulik wrote: -1 that's what the visible: pmSource.data[Battery][Has Cumulative] is for. there just used to be a bug where that property wasn't created in the first place if no battery was present leading to an exception causing the item to remain visible. Antonis Tsiapaliokas wrote: What about checking if the pmSource.data[Battery][Has Cumulative] has been set properly, and if not, then using the pmSource.data[Battery][Has Battery]? I have update my patch with the above change... Kai Uwe Broulik wrote: I don't quite get it. Theoretically Has Cumulative is true when there is a power supply battery and it is false if there isn't. I suppose you're running latest plasma-workspace and powerdevil? I used to have this issue on my desktop but fixed it and it's gone here. Could you paste the output of upower -d for your machine, and also the value of Has Cumulative in plasmaengineexplorer? Antonis Tsiapaliokas wrote: When i wrote this patch(2 days ago), plasmaengineexplorer didn't had the Has Cumulative. Today i update my setup and plasmaenginexplorer has a value for the Has Cumulative. Yes, i am running the latest plasma-workspace and powerdevil. So since none of us can reproduce the issue anymore, and you have fixed the issue with the Has Cumulative, i guess we can discard this review. Right? Kai Uwe Broulik wrote: That one [1] fixed it; sorry for the confusion, could be closed then :) [1] http://quickgit.kde.org/?p=plasma-workspace.gita=commith=49c425a80eca011cff20d2af47477f6ffb76e3bf Np :) - Antonis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79321 --- On April 24, 2015, 6:22 p.m., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:22 p.m.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 7:02 p.m.) Status -- This change has been discarded. Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
On April 22, 2015, 3:46 vorm., Kai Uwe Broulik wrote: -1 that's what the visible: pmSource.data[Battery][Has Cumulative] is for. there just used to be a bug where that property wasn't created in the first place if no battery was present leading to an exception causing the item to remain visible. Antonis Tsiapaliokas wrote: What about checking if the pmSource.data[Battery][Has Cumulative] has been set properly, and if not, then using the pmSource.data[Battery][Has Battery]? I have update my patch with the above change... Kai Uwe Broulik wrote: I don't quite get it. Theoretically Has Cumulative is true when there is a power supply battery and it is false if there isn't. I suppose you're running latest plasma-workspace and powerdevil? I used to have this issue on my desktop but fixed it and it's gone here. Could you paste the output of upower -d for your machine, and also the value of Has Cumulative in plasmaengineexplorer? Antonis Tsiapaliokas wrote: When i wrote this patch(2 days ago), plasmaengineexplorer didn't had the Has Cumulative. Today i update my setup and plasmaenginexplorer has a value for the Has Cumulative. Yes, i am running the latest plasma-workspace and powerdevil. So since none of us can reproduce the issue anymore, and you have fixed the issue with the Has Cumulative, i guess we can discard this review. Right? That one [1] fixed it; sorry for the confusion, could be closed then :) [1] http://quickgit.kde.org/?p=plasma-workspace.gita=commith=49c425a80eca011cff20d2af47477f6ffb76e3bf - Kai Uwe --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79321 --- On April 24, 2015, 6:22 nachm., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 24, 2015, 6:22 nachm.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79321 --- -1 that's what the visible: pmSource.data[Battery][Has Cumulative] is for. there just used to be a bug where that property wasn't created in the first place if no battery was present leading to an exception causing the item to remain visible. - Kai Uwe Broulik On April 21, 2015, 9:18 nachm., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 21, 2015, 9:18 nachm.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79311 --- lookandfeel/contents/components/InfoPane.qml (line 47) https://git.reviewboard.kde.org/r/123459/#comment54152 there's a visible here. If your two lines are needed, this isn't. Or this should be changed to the battery.hasBattery instead. - David Edmundson On April 21, 2015, 9:18 p.m., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated April 21, 2015, 9:18 p.m.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123459: Lockscreen: It shouldn't show the battery information on system which they don't have a battery
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/#review79312 --- Ship it! Ship It! - Lukáš Tinkl On Dub. 21, 2015, 11:18 odp., Antonis Tsiapaliokas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123459/ --- (Updated Dub. 21, 2015, 11:18 odp.) Review request for Plasma. Bugs: 346441 https://bugs.kde.org/show_bug.cgi?id=346441 Repository: plasma-workspace Description --- The battery information shouldn't be shown on systems which they don't have battery (desktops/laptop without battery). Diffs - lookandfeel/contents/components/InfoPane.qml 18739ad Diff: https://git.reviewboard.kde.org/r/123459/diff/ Testing --- File Attachments no battery after https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/d3e0b27b-e695-46a2-b56a-dc60c9679309__no_battery_after.png no battery before https://git.reviewboard.kde.org/media/uploaded/files/2015/04/21/884a20d3-852c-4b1d-a588-7bb47e725011__no_battery_before.png Thanks, Antonis Tsiapaliokas ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
