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

Issue 1530533002: Add filters for user service. (Closed)

Created:
5 years ago by iannucci
Modified:
5 years ago
Reviewers:
dnj, Vadim Sh., estaab, martiniss
CC:
chromium-reviews, infra-reviews+luci-gae_chromium.org
Base URL:
https://github.com/luci/gae.git@master
Target Ref:
refs/heads/master
Project:
luci-gae
Visibility:
Public.

Description

Add filters for user service. This brings the user service up to parity with the rest. R=dnj@chromium.org, estaab@chromium.org, martiniss@chromium.org, vadimsh@chromium.org BUG= Committed: https://github.com/luci/gae/commit/6014627a303145614fb882022e7a9656a82b8408

Patch Set 1 #

Total comments: 2

Patch Set 2 : bug #

Unified diffs Side-by-side diffs Delta from patch set Stats (+205 lines, -4 lines) Patch
M filter/count/count_test.go View 2 chunks +18 lines, -0 lines 0 comments Download
A filter/count/user.go View 1 1 chunk +76 lines, -0 lines 0 comments Download
A filter/featureBreaker/user.go View 1 chunk +66 lines, -0 lines 0 comments Download
M service/user/context.go View 2 chunks +45 lines, -4 lines 0 comments Download

Messages

Total messages: 12 (3 generated)
iannucci
5 years ago (2015-12-15 00:37:05 UTC) #1
iannucci
On 2015/12/15 at 00:37:05, iannucci wrote: > it's mostly copypasta :) Probably should think about ...
5 years ago (2015-12-15 00:38:28 UTC) #2
iannucci
On 2015/12/15 at 00:38:28, iannucci wrote: > On 2015/12/15 at 00:37:05, iannucci wrote: > > ...
5 years ago (2015-12-15 00:38:42 UTC) #3
iannucci
(ping)
5 years ago (2015-12-15 18:54:45 UTC) #4
dnj
lgtm w/ comment https://chromiumcodereview.appspot.com/1530533002/diff/1/filter/count/user.go File filter/count/user.go (right): https://chromiumcodereview.appspot.com/1530533002/diff/1/filter/count/user.go#newcode32 filter/count/user.go:32: u.c.Counter.up() Why not named "Current"?
5 years ago (2015-12-15 18:59:30 UTC) #5
dnj
lgtm w/ comment https://chromiumcodereview.appspot.com/1530533002/diff/1/filter/count/user.go File filter/count/user.go (right): https://chromiumcodereview.appspot.com/1530533002/diff/1/filter/count/user.go#newcode32 filter/count/user.go:32: u.c.Counter.up() Why not named "Current"?
5 years ago (2015-12-15 18:59:30 UTC) #6
iannucci
This is exactly why I filed https://github.com/luci/gae/issues/30 :( https://codereview.chromium.org/1530533002/diff/1/filter/count/user.go File filter/count/user.go (right): https://codereview.chromium.org/1530533002/diff/1/filter/count/user.go#newcode32 filter/count/user.go:32: u.c.Counter.up() ...
5 years ago (2015-12-15 19:12:21 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530533002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530533002/20001
5 years ago (2015-12-15 19:12:41 UTC) #10
commit-bot: I haz the power
5 years ago (2015-12-15 19:15:20 UTC) #12
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://github.com/luci/gae/commit/6014627a303145614fb882022e7a9656a82b8408

Powered by Google App Engine
This is Rietveld 408576698