Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2597343002:
Add a histogram to track the type of documents printed (HTML vs PDF) (Closed)
DescriptionAdd a histogram to track the type of documents printed (HTML vs PDF)
For print preview UI redesign, design team has requested tracking of
number of HTML vs PDF prints as the UI elements for each case are
different. This adds a histogram to track HTML vs PDF prints.
BUG=
Committed: https://crrev.com/d34d954730840b4661d9bf8bc972a3d6b4552995
Cr-Commit-Position: refs/heads/master@{#440917}
Patch Set 1 #Patch Set 2 : Reduce diff #
Total comments: 4
Patch Set 3 : Add documentation #
Total comments: 1
Messages
Total messages: 30 (20 generated)
The CQ bit was checked by rbpotter@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 ========== Add a histogram to track the type of documents printed (HTML vs PDF) For print preview UI redesign, design team has requested tracking of number of HTML vs PDF prints as the UI elements for each case are different. This adds a histogram to track HTML vs PDF prints. BUG= ========== to ========== Add a histogram to track the type of documents printed (HTML vs PDF) For print preview UI redesign, design team has requested tracking of number of HTML vs PDF prints as the UI elements for each case are different. This adds a histogram to track HTML vs PDF prints. BUG= ==========
rbpotter@chromium.org changed reviewers: + isherman@chromium.org, vitalybuka@chromium.org
The CQ bit was checked by rbpotter@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...
On 2016/12/28 22:39:23, rbpotter wrote: Note: Not urgent - can wait until after holidays if needed.
lgtm
https://codereview.chromium.org/2597343002/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right): https://codereview.chromium.org/2597343002/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/print_preview/print_preview_handler.cc:148: enum PrintDocumentTypeBuckets { Please document that this enum is used to back an UMA histogram, and should therefore be treated as append-only. https://codereview.chromium.org/2597343002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2597343002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:50210: + <summary>Track type of documents printed (HTML vs PDF).</summary> Please document precisely when this metric is recorded, i.e. right after a document is printed.
Sorry, meant to say: Metrics LGTM with my comments addressed. Thanks!
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 rbpotter@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...
https://codereview.chromium.org/2597343002/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right): https://codereview.chromium.org/2597343002/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/print_preview/print_preview_handler.cc:148: enum PrintDocumentTypeBuckets { On 2016/12/28 23:25:53, Ilya Sherman (Away De.29-Ja.4) wrote: > Please document that this enum is used to back an UMA histogram, and should > therefore be treated as append-only. Done. Made similar notes for the two enums above, which also back UMA histograms. https://codereview.chromium.org/2597343002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2597343002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:50210: + <summary>Track type of documents printed (HTML vs PDF).</summary> On 2016/12/28 23:25:53, Ilya Sherman (Away De.29-Ja.4) wrote: > Please document precisely when this metric is recorded, i.e. right after a > document is printed. Done. It actually occurs right after the request; have added this in the summary.
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 rbpotter@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from isherman@chromium.org, vitalybuka@chromium.org Link to the patchset: https://codereview.chromium.org/2597343002/#ps40001 (title: "Add documentation")
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": 40001, "attempt_start_ts": 1482972888395050,
"parent_rev": "170236a519483f60ff4a846baeef618f39fd66be", "commit_rev":
"c105348311c05f0d17f6cf68bd0aefbc1b28e05c"}
Message was sent while issue was closed.
Description was changed from ========== Add a histogram to track the type of documents printed (HTML vs PDF) For print preview UI redesign, design team has requested tracking of number of HTML vs PDF prints as the UI elements for each case are different. This adds a histogram to track HTML vs PDF prints. BUG= ========== to ========== Add a histogram to track the type of documents printed (HTML vs PDF) For print preview UI redesign, design team has requested tracking of number of HTML vs PDF prints as the UI elements for each case are different. This adds a histogram to track HTML vs PDF prints. BUG= Review-Url: https://codereview.chromium.org/2597343002 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add a histogram to track the type of documents printed (HTML vs PDF) For print preview UI redesign, design team has requested tracking of number of HTML vs PDF prints as the UI elements for each case are different. This adds a histogram to track HTML vs PDF prints. BUG= Review-Url: https://codereview.chromium.org/2597343002 ========== to ========== Add a histogram to track the type of documents printed (HTML vs PDF) For print preview UI redesign, design team has requested tracking of number of HTML vs PDF prints as the UI elements for each case are different. This adds a histogram to track HTML vs PDF prints. BUG= Committed: https://crrev.com/d34d954730840b4661d9bf8bc972a3d6b4552995 Cr-Commit-Position: refs/heads/master@{#440917} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/d34d954730840b4661d9bf8bc972a3d6b4552995 Cr-Commit-Position: refs/heads/master@{#440917}
Message was sent while issue was closed.
thestig@chromium.org changed reviewers: + thestig@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2597343002/diff/40001/chrome/browser/ui/webui... File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right): https://codereview.chromium.org/2597343002/diff/40001/chrome/browser/ui/webui... chrome/browser/ui/webui/print_preview/print_preview_handler.cc:155: HTML_DOCUMENT = 0, Maybe we should see how often users print images too? i.e. load http://domain/foo.jpeg and print that directly. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
