Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2218003005:
Don't log GetScreenBrightnessPercent() errors in VMs (Closed)
|
Created:
4 years, 4 months ago by norvez Modified:
4 years, 4 months ago CC:
chromium-reviews, derat+watch_chromium.org, hashimoto+watch_chromium.org, oshima+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@crbug632303 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't log GetScreenBrightnessPercent() errors in VMs
No screens on GCE VMs; don't log GetScreenBrightnessPercent errors in that case.
BUG=chromium:585514
TEST=Ran manually in QEMU, no GetScreenBrightnessPercent
error in power_manager_client.cc
Committed: https://crrev.com/e8ddb0e019f79815697d1603cc6266143fdb8704
Cr-Commit-Position: refs/heads/master@{#413103}
Patch Set 1 #
Total comments: 1
Patch Set 2 : use isRunningOnChromeOSVM() method #Patch Set 3 : fix CL description #Patch Set 4 : method name has changed #Patch Set 5 : update patch dependency #Patch Set 6 : update patch dependency #Patch Set 7 : update patch dependency #Patch Set 8 : update patch dependency #Patch Set 9 : update patch dependency #Patch Set 10 : update patch dependency #Messages
Total messages: 46 (33 generated)
The CQ bit was checked by norvez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
norvez@chromium.org changed reviewers: + achuith@chromium.org, derat@chromium.org, xdai@chromium.org
This CL is a rehash of https://codereview.chromium.org/1981383002/ based on the change https://codereview.chromium.org/2218703006. It should fix the bogus error messages about GetScreenBrightnessPercent
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
looks okay, but mind correcting the commit message? we still call it; we just don't log an error if it fails. :-)
https://codereview.chromium.org/2218003005/diff/1/chromeos/dbus/power_manager... File chromeos/dbus/power_manager_client.cc (right): https://codereview.chromium.org/2218003005/diff/1/chromeos/dbus/power_manager... chromeos/dbus/power_manager_client.cc:467: hwid != chromeos::system::kHardwareClassValueVM) { Why not just have a method like IsRunningOnVM() instead?
Thanks for the feedback! Somehow git cl upload seems to have lost the update to the commit message, I'll edit the issue by hand.
Description was changed from ========== Don't call GetScreenBrightnessPercent in VMs No screens on GCE VMs, don't call GetScreenBrightnessPercent in that case. BUG=chromium:585514 TEST=Ran manually in QEMU, no GetScreenBrightnessPercent error in power_manager_client.cc ========== to ========== Don't log GetScreenBrightnessPercent() errors in VMs No screens on GCE VMs, don't log GetScreenBrightnessPercent errors in that case. BUG=chromium:585514 TEST=Ran manually in QEMU, no GetScreenBrightnessPercent error in power_manager_client.cc ==========
CL description fixed as per review comments
The CQ bit was checked by norvez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from
==========
Don't log GetScreenBrightnessPercent() errors in VMs
No screens on GCE VMs, don't log GetScreenBrightnessPercent errors in
that case.
BUG=chromium:585514
TEST=Ran manually in QEMU, no GetScreenBrightnessPercent error in
power_manager_client.cc
==========
to
==========
Don't log GetScreenBrightnessPercent() errors in VMs
No screens on GCE VMs; don't log GetScreenBrightnessPercent errors in that case.
BUG=chromium:585514
TEST=Ran manually in QEMU, no GetScreenBrightnessPercent
error in power_manager_client.cc
==========
Fixed patchset after review.
The CQ bit was checked by norvez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by norvez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by norvez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by norvez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by norvez@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from achuith@chromium.org, xdai@chromium.org, derat@chromium.org Link to the patchset: https://codereview.chromium.org/2218003005/#ps180001 (title: "update patch dependency")
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 2218703006 Patch 180001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
The CQ bit was checked by norvez@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from
==========
Don't log GetScreenBrightnessPercent() errors in VMs
No screens on GCE VMs; don't log GetScreenBrightnessPercent errors in that case.
BUG=chromium:585514
TEST=Ran manually in QEMU, no GetScreenBrightnessPercent
error in power_manager_client.cc
==========
to
==========
Don't log GetScreenBrightnessPercent() errors in VMs
No screens on GCE VMs; don't log GetScreenBrightnessPercent errors in that case.
BUG=chromium:585514
TEST=Ran manually in QEMU, no GetScreenBrightnessPercent
error in power_manager_client.cc
==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Description was changed from
==========
Don't log GetScreenBrightnessPercent() errors in VMs
No screens on GCE VMs; don't log GetScreenBrightnessPercent errors in that case.
BUG=chromium:585514
TEST=Ran manually in QEMU, no GetScreenBrightnessPercent
error in power_manager_client.cc
==========
to
==========
Don't log GetScreenBrightnessPercent() errors in VMs
No screens on GCE VMs; don't log GetScreenBrightnessPercent errors in that case.
BUG=chromium:585514
TEST=Ran manually in QEMU, no GetScreenBrightnessPercent
error in power_manager_client.cc
Committed: https://crrev.com/e8ddb0e019f79815697d1603cc6266143fdb8704
Cr-Commit-Position: refs/heads/master@{#413103}
==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/e8ddb0e019f79815697d1603cc6266143fdb8704 Cr-Commit-Position: refs/heads/master@{#413103} |
