Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2956333002:
[Sync] Enable UserEvents by default. (Closed)
|
Created:
3 years, 5 months ago by skym Modified:
3 years, 5 months ago Reviewers:
Patrick Noland CC:
chromium-reviews, sync-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[Sync] Enable UserEvents by default.
Given we have approval to launching general EventLog machinery and each
actual EventLog use case (currently only translate) has their own
gating features, this seems reasonable to turn off by default. Leaving
the feature in the code such that we will have a kill switch if
anything goes wrong.
BUG=701032
Review-Url: https://codereview.chromium.org/2956333002
Cr-Commit-Position: refs/heads/master@{#484366}
Committed: https://chromium.googlesource.com/chromium/src/+/afe0f2c3cac407b2c28cf01f05d67a509418f5e7
Patch Set 1 #Patch Set 2 : Rebase #Patch Set 3 : Fixed failing unit tests. #Patch Set 4 : Maybe iOS will work this time? #
Messages
Total messages: 31 (21 generated)
The CQ bit was checked by skym@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...
Description was changed from ========== [Sync] Enable UserEvents by default. BUG=701032 ========== to ========== [Sync] Enable UserEvents by default. Given we have approval to launching general EventLog machinery and each actual EventLog use case (currently only translate) has their own gating features, this seems reasonable to turn off by default. Leaving the feature in the code such that we will have a kill switch if anything goes wrong. BUG=701032 ==========
skym@chromium.org changed reviewers: + pnoland@chromium.org
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
I think the trybot failures are real and related?
On 2017/06/28 22:45:42, Patrick Noland wrote: > I think the trybot failures are real and related? Yeah, this branch/patch should have depended upon https://codereview.chromium.org/2948303002/
The CQ bit was checked by skym@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: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by skym@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: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
I was wrong Patrick, this change did need an actual fix for the unit tests! I'm pretty sure the iOS failure still left isn't me, but part of crbug.com/738092 and crbug.com/739387 PTAL.
On 2017/07/05 18:56:34, skym wrote: > I was wrong Patrick, this change did need an actual fix for the unit tests! I'm > pretty sure the iOS failure still left isn't me, but part of crbug.com/738092 > and crbug.com/739387 > > PTAL. The ios failure might be because you need to add datatypes.push_back(syncer::USER_EVENTS); to ios_chrome_profile_sync_service_factory_unittest.cc but the error message isn't clear enough to be sure.
The CQ bit was checked by skym@chromium.org to run a CQ dry run
Added to ios tests
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by skym@chromium.org
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": 60001, "attempt_start_ts": 1499289784796980,
"parent_rev": "36a816e7d1de8c3e1a2a720b05743ceb8405653d", "commit_rev":
"afe0f2c3cac407b2c28cf01f05d67a509418f5e7"}
Message was sent while issue was closed.
Description was changed from ========== [Sync] Enable UserEvents by default. Given we have approval to launching general EventLog machinery and each actual EventLog use case (currently only translate) has their own gating features, this seems reasonable to turn off by default. Leaving the feature in the code such that we will have a kill switch if anything goes wrong. BUG=701032 ========== to ========== [Sync] Enable UserEvents by default. Given we have approval to launching general EventLog machinery and each actual EventLog use case (currently only translate) has their own gating features, this seems reasonable to turn off by default. Leaving the feature in the code such that we will have a kill switch if anything goes wrong. BUG=701032 Review-Url: https://codereview.chromium.org/2956333002 Cr-Commit-Position: refs/heads/master@{#484366} Committed: https://chromium.googlesource.com/chromium/src/+/afe0f2c3cac407b2c28cf01f05d6... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/afe0f2c3cac407b2c28cf01f05d6...
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2968133002/ by lod@chromium.org. The reason for reverting is: Hello skym, unfortunately the answer to "Maybe iOS will work this time" was no :( It's breaking the downstream ios_chrome_ui_egtests . One such failure here: https://uberchromegw.corp.google.com/i/internal.bling.main/builders/ipad10-de... . |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
