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

Issue 2892553003: Initial proto for language detection. (Closed)

Created:
3 years, 7 months ago by renjieliu1
Modified:
3 years, 7 months ago
Reviewers:
napper, skym, amoylan
CC:
chromium-reviews, sync-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 14

Patch Set 2 : update proto #

Total comments: 4

Patch Set 3 : change proportion to is_reliable #

Patch Set 4 : change proportion to is_reliable #

Total comments: 2

Patch Set 5 : add comment #

Total comments: 6

Patch Set 6 : update #

Total comments: 2

Patch Set 7 : fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -1 line) Patch
M components/sync/protocol/proto_visitors.h View 1 2 3 2 chunks +11 lines, -0 lines 0 comments Download
M components/sync/protocol/user_event_specifics.proto View 1 2 3 4 5 6 2 chunks +22 lines, -1 line 0 comments Download

Messages

Total messages: 30 (13 generated)
renjieliu1
3 years, 7 months ago (2017-05-18 04:41:51 UTC) #3
napper
https://codereview.chromium.org/2892553003/diff/1/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/1/components/sync/protocol/user_event_specifics.proto#newcode16 components/sync/protocol/user_event_specifics.proto:16: // Language detection output Add . to comment https://codereview.chromium.org/2892553003/diff/1/components/sync/protocol/user_event_specifics.proto#newcode19 ...
3 years, 7 months ago (2017-05-18 04:55:07 UTC) #4
renjieliu1
thanks for the review! https://codereview.chromium.org/2892553003/diff/1/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/1/components/sync/protocol/user_event_specifics.proto#newcode16 components/sync/protocol/user_event_specifics.proto:16: // Language detection output On ...
3 years, 7 months ago (2017-05-18 05:48:22 UTC) #6
renjieliu1
thanks for the review!
3 years, 7 months ago (2017-05-18 05:48:24 UTC) #7
napper
https://codereview.chromium.org/2892553003/diff/20001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/20001/components/sync/protocol/user_event_specifics.proto#newcode22 components/sync/protocol/user_event_specifics.proto:22: optional int32 proportion = 2; As discussed offline, switch ...
3 years, 7 months ago (2017-05-18 06:02:14 UTC) #9
renjieliu1
https://codereview.chromium.org/2892553003/diff/20001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/20001/components/sync/protocol/user_event_specifics.proto#newcode22 components/sync/protocol/user_event_specifics.proto:22: optional int32 proportion = 2; On 2017/05/18 06:02:13, napper ...
3 years, 7 months ago (2017-05-18 06:08:57 UTC) #13
napper
lgtm
3 years, 7 months ago (2017-05-18 06:13:06 UTC) #15
amoylan
lgtm https://codereview.chromium.org/2892553003/diff/60001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/60001/components/sync/protocol/user_event_specifics.proto#newcode28 components/sync/protocol/user_event_specifics.proto:28: optional string adopted_language = 2; Please add a ...
3 years, 7 months ago (2017-05-18 06:36:21 UTC) #16
renjieliu1
https://codereview.chromium.org/2892553003/diff/60001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/60001/components/sync/protocol/user_event_specifics.proto#newcode28 components/sync/protocol/user_event_specifics.proto:28: optional string adopted_language = 2; On 2017/05/18 06:36:21, amoylan ...
3 years, 7 months ago (2017-05-18 06:58:27 UTC) #17
renjieliu1
Hi Sky, I'm implementing chrome language event logging. I followed your sample cl here: https://codereview.chromium.org/2875203002/ ...
3 years, 7 months ago (2017-05-18 07:08:40 UTC) #19
skym
lgtm https://codereview.chromium.org/2892553003/diff/80001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/80001/components/sync/protocol/user_event_specifics.proto#newcode25 components/sync/protocol/user_event_specifics.proto:25: // Top n languages. Typically we just log ...
3 years, 7 months ago (2017-05-19 03:22:35 UTC) #20
napper
https://codereview.chromium.org/2892553003/diff/80001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/80001/components/sync/protocol/user_event_specifics.proto#newcode29 components/sync/protocol/user_event_specifics.proto:29: optional string adopted_language = 2; What if we only ...
3 years, 7 months ago (2017-05-19 03:30:59 UTC) #21
renjieliu1
thanks for the review! https://codereview.chromium.org/2892553003/diff/80001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/80001/components/sync/protocol/user_event_specifics.proto#newcode25 components/sync/protocol/user_event_specifics.proto:25: // Top n languages. Typically ...
3 years, 7 months ago (2017-05-19 04:10:17 UTC) #22
napper
https://codereview.chromium.org/2892553003/diff/100001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/100001/components/sync/protocol/user_event_specifics.proto#newcode38 components/sync/protocol/user_event_specifics.proto:38: // It will be stored only it's different from ...
3 years, 7 months ago (2017-05-19 04:16:55 UTC) #23
renjieliu1
https://codereview.chromium.org/2892553003/diff/100001/components/sync/protocol/user_event_specifics.proto File components/sync/protocol/user_event_specifics.proto (right): https://codereview.chromium.org/2892553003/diff/100001/components/sync/protocol/user_event_specifics.proto#newcode38 components/sync/protocol/user_event_specifics.proto:38: // It will be stored only it's different from ...
3 years, 7 months ago (2017-05-19 04:37:12 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2892553003/120001
3 years, 7 months ago (2017-05-19 05:37:48 UTC) #27
commit-bot: I haz the power
3 years, 7 months ago (2017-05-19 06:58:25 UTC) #30
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/0b301f711dbc495c3b552d083d0e...

Powered by Google App Engine
This is Rietveld 408576698