Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2620343004:
chromeos: Document Power.ConnectedChargingPorts histogram. (Closed)
|
Created:
3 years, 11 months ago by Daniel Erat Modified:
3 years, 11 months ago Reviewers:
Steven Holte CC:
chromium-reviews, asvitkine+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionchromeos: Document Power.ConnectedChargingPorts histogram.
Document a new Power.ConnectedChargingPorts histogram added
by https://chromium-review.googlesource.com/c/426381/ .
BUG=674338
Review-Url: https://codereview.chromium.org/2620343004
Cr-Commit-Position: refs/heads/master@{#443902}
Committed: https://chromium.googlesource.com/chromium/src/+/769bfbde2134ce75abd6c3323b341e30a536aba1
Patch Set 1 #
Total comments: 5
Patch Set 2 : PowerChargingPorts -> PowerConnectedChargingPorts #Messages
Total messages: 18 (13 generated)
The CQ bit was checked by derat@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...
derat@chromium.org changed reviewers: + holte@chromium.org
https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:100158: +<enum name="PowerChargingPorts" type="int"> would "PowerConnectedChargingPorts" be better? just "ConnectedChargingPorts"? https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:100160: + Connected charging ports on Chrome OS, as reported by the kernel. i put the details about this enum inside of the histogram's summary instead of here, as the former appears to be displayed on the dashboard while enums' summaries aren't (unless i was looking in the wrong place).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:100158: +<enum name="PowerChargingPorts" type="int"> On 2017/01/12 18:38:27, Daniel Erat wrote: > would "PowerConnectedChargingPorts" be better? just "ConnectedChargingPorts"? PowerConnectedChargingPorts might be slightly better. https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:100160: + Connected charging ports on Chrome OS, as reported by the kernel. On 2017/01/12 18:38:27, Daniel Erat wrote: > i put the details about this enum inside of the histogram's summary instead of > here, as the former appears to be displayed on the dashboard while enums' > summaries aren't (unless i was looking in the wrong place). Yes, I don't think these are currently displayed, so putting it in the histograms description seems fine.
https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2620343004/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:100158: +<enum name="PowerChargingPorts" type="int"> On 2017/01/12 21:23:23, Steven Holte wrote: > On 2017/01/12 18:38:27, Daniel Erat wrote: > > would "PowerConnectedChargingPorts" be better? just "ConnectedChargingPorts"? > > PowerConnectedChargingPorts might be slightly better. done. thanks!
The CQ bit was checked by derat@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 derat@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from holte@chromium.org Link to the patchset: https://codereview.chromium.org/2620343004/#ps20001 (title: "PowerChargingPorts -> PowerConnectedChargingPorts")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484578072324380,
"parent_rev": "43c154f97c004706a96ed0f185d6232b12824c76", "commit_rev":
"769bfbde2134ce75abd6c3323b341e30a536aba1"}
Message was sent while issue was closed.
Description was changed from ========== chromeos: Document Power.ConnectedChargingPorts histogram. Document a new Power.ConnectedChargingPorts histogram added by https://chromium-review.googlesource.com/c/426381/ . BUG=674338 ========== to ========== chromeos: Document Power.ConnectedChargingPorts histogram. Document a new Power.ConnectedChargingPorts histogram added by https://chromium-review.googlesource.com/c/426381/ . BUG=674338 Review-Url: https://codereview.chromium.org/2620343004 Cr-Commit-Position: refs/heads/master@{#443902} Committed: https://chromium.googlesource.com/chromium/src/+/769bfbde2134ce75abd6c3323b34... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/769bfbde2134ce75abd6c3323b34... |
