Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(351)

Issue 1650983002: Add an IsBootstrapped method to the SiteEngagementService. (Closed)

Created:
4 years, 10 months ago by calamity
Modified:
4 years, 10 months ago
Reviewers:
dominickn
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@adjust_constants
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add an IsBootstrapped method to the SiteEngagementService. This CL allows the SiteEngagementService to report whether it has enough data to be considered 'useful'. This is currently defined as 30 engagement points in the engagement system. BUG=464234 Committed: https://crrev.com/7c07c7a1722a02d1664040e231858208b0d4f86b Cr-Commit-Position: refs/heads/master@{#373656}

Patch Set 1 #

Total comments: 2

Patch Set 2 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -0 lines) Patch
M chrome/browser/engagement/site_engagement_service.h View 1 4 chunks +11 lines, -0 lines 0 comments Download
M chrome/browser/engagement/site_engagement_service.cc View 1 4 chunks +11 lines, -0 lines 0 comments Download
M chrome/browser/engagement/site_engagement_service_unittest.cc View 1 1 chunk +23 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 12 (6 generated)
calamity
4 years, 10 months ago (2016-02-04 02:10:11 UTC) #3
dominickn
lgtm % nit https://codereview.chromium.org/1650983002/diff/1/chrome/browser/engagement/site_engagement_service.cc File chrome/browser/engagement/site_engagement_service.cc (right): https://codereview.chromium.org/1650983002/diff/1/chrome/browser/engagement/site_engagement_service.cc#newcode129 chrome/browser/engagement/site_engagement_service.cc:129: 30, // BOOTSTRAP_POINTS Nit: as discussed ...
4 years, 10 months ago (2016-02-04 03:11:24 UTC) #4
calamity
https://codereview.chromium.org/1650983002/diff/1/chrome/browser/engagement/site_engagement_service.cc File chrome/browser/engagement/site_engagement_service.cc (right): https://codereview.chromium.org/1650983002/diff/1/chrome/browser/engagement/site_engagement_service.cc#newcode129 chrome/browser/engagement/site_engagement_service.cc:129: 30, // BOOTSTRAP_POINTS On 2016/02/04 03:11:24, dominickn wrote: > ...
4 years, 10 months ago (2016-02-04 23:09:09 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1650983002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1650983002/20001
4 years, 10 months ago (2016-02-04 23:13:31 UTC) #8
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 10 months ago (2016-02-04 23:24:20 UTC) #10
commit-bot: I haz the power
4 years, 10 months ago (2016-02-04 23:26:12 UTC) #12
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/7c07c7a1722a02d1664040e231858208b0d4f86b
Cr-Commit-Position: refs/heads/master@{#373656}

Powered by Google App Engine
This is Rietveld 408576698