Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2544093002:
Add UMA stats for Scout initialization logic (Closed)
DescriptionAdd UMA stats for Scout initialization logic
BUG=665570
Committed: https://crrev.com/5420cc0f557f605f9f2ac1860eaf6bb783b7ea87
Cr-Commit-Position: refs/heads/master@{#436636}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Add more comments to reasons enum #
Total comments: 4
Patch Set 3 : Sync #Patch Set 4 : Include metric for seeing first interstitial #Patch Set 5 : Sync #
Messages
Total messages: 35 (23 generated)
The CQ bit was checked by lpz@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...
lpz@chromium.org changed reviewers: + jialiul@chromium.org, nparker@chromium.org
vakh@chromium.org changed reviewers: + vakh@chromium.org
lgtm https://codereview.chromium.org/2544093002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/safe_browsing_prefs.cc (right): https://codereview.chromium.org/2544093002/diff/1/components/safe_browsing_db... components/safe_browsing_db/safe_browsing_prefs.cc:21: FORCE_SCOUT_FLAG_TRUE = 0, It would be good to have the strings that you added in histograms here also as comments. That would make understanding these enum values much much easier.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2544093002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/safe_browsing_prefs.cc (right): https://codereview.chromium.org/2544093002/diff/1/components/safe_browsing_db... components/safe_browsing_db/safe_browsing_prefs.cc:20: enum ScoutTransitionReason { nit: Maybe add a comment to remind others not to reorder this enum.
The CQ bit was checked by lpz@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...
lpz@chromium.org changed reviewers: + jwd@chromium.org
Thanks! +jwd for histograms.xml https://codereview.chromium.org/2544093002/diff/1/components/safe_browsing_db... File components/safe_browsing_db/safe_browsing_prefs.cc (right): https://codereview.chromium.org/2544093002/diff/1/components/safe_browsing_db... components/safe_browsing_db/safe_browsing_prefs.cc:20: enum ScoutTransitionReason { On 2016/12/01 22:31:11, Jialiu Lin wrote: > nit: Maybe add a comment to remind others not to reorder this enum. Done. https://codereview.chromium.org/2544093002/diff/1/components/safe_browsing_db... components/safe_browsing_db/safe_browsing_prefs.cc:21: FORCE_SCOUT_FLAG_TRUE = 0, On 2016/12/01 21:47:07, vakh (Varun Khaneja) wrote: > It would be good to have the strings that you added in histograms here also as > comments. That would make understanding these enum values much much easier. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:100333: - <int value="196" label="R01"/> This looks like an unintentional change... maybe need to rebase? https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:53073: + Tracks reasons for the Extended Reporting preference transition, such as a So is this recorded only when a state transition occurs? like, if I restart twice in a row, chances are the second time there'll be no change? This seems pretty useful for seeing how users are moving. Cool
The CQ bit was checked by lpz@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/2544093002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:100333: - <int value="196" label="R01"/> On 2016/12/03 18:06:37, Nathan Parker wrote: > This looks like an unintentional change... maybe need to rebase? Done. https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:53073: + Tracks reasons for the Extended Reporting preference transition, such as a On 2016/12/03 18:06:36, Nathan Parker wrote: > So is this recorded only when a state transition occurs? like, if I restart > twice in a row, chances are the second time there'll be no change? > > This seems pretty useful for seeing how users are moving. Cool yep, that's correct, only on transition. The exception is the one called "control" which will be incremented on each startup for users in that group. But control also has its own transitions for rollbacks etc that will only be recorded once.
https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:100333: - <int value="196" label="R01"/> On 2016/12/03 18:06:37, Nathan Parker wrote: > This looks like an unintentional change... maybe need to rebase? Done. https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2544093002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:53073: + Tracks reasons for the Extended Reporting preference transition, such as a On 2016/12/03 18:06:36, Nathan Parker wrote: > So is this recorded only when a state transition occurs? like, if I restart > twice in a row, chances are the second time there'll be no change? > > This seems pretty useful for seeing how users are moving. Cool yep, that's correct, only on transition. The exception is the one called "control" which will be incremented on each startup for users in that group. But control also has its own transitions for rollbacks etc that will only be recorded once.
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 lpz@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...
Also added tracking of users seeing their first interstitial and entering Scout Group that way.
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 lpz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from vakh@chromium.org, jialiul@chromium.org, nparker@chromium.org, jwd@chromium.org Link to the patchset: https://codereview.chromium.org/2544093002/#ps80001 (title: "Sync")
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": 80001, "attempt_start_ts": 1481041603894630,
"parent_rev": "65e54ca47db906eb43c1ec732d54b21047368460", "commit_rev":
"10fa25d8241623ebc6f2c16118dbdc8f9f517f0e"}
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Add UMA stats for Scout initialization logic BUG=665570 ========== to ========== Add UMA stats for Scout initialization logic BUG=665570 Committed: https://crrev.com/5420cc0f557f605f9f2ac1860eaf6bb783b7ea87 Cr-Commit-Position: refs/heads/master@{#436636} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/5420cc0f557f605f9f2ac1860eaf6bb783b7ea87 Cr-Commit-Position: refs/heads/master@{#436636} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
