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

Issue 1259463002: Add BoolFlag for rawdatastore. (Closed)

Created:
5 years, 5 months ago by iannucci
Modified:
5 years, 5 months ago
CC:
chromium-reviews
Base URL:
https://github.com/luci/gae.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Add Toggle for rawdatastore. This will be used to control cacheability for entities, and allows setting a default value in the tag without ambiguity (since the 'zero-value' for bools is false, a simple bool wouldn't provide a way to override defaulted-true field). R=dnj@google.com, vadimsh@chromium.org, dnj@chromium.org, estaab@chromium.org BUG= Committed: https://github.com/luci/gae/commit/92ee98fa28f06f5bdb297d6974ca085e566effff

Patch Set 1 #

Total comments: 1

Patch Set 2 : rename to toggle #

Total comments: 4

Patch Set 3 : clarified #

Patch Set 4 : improve error #

Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -4 lines) Patch
M service/rawdatastore/datastore_impl.go View 1 2 3 3 chunks +20 lines, -1 line 0 comments Download
M service/rawdatastore/datastore_test.go View 1 2 3 2 chunks +30 lines, -1 line 0 comments Download
M service/rawdatastore/properties.go View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
M service/rawdatastore/reflect.go View 1 1 chunk +3 lines, -2 lines 0 comments Download
M service/rawdatastore/types.go View 1 2 chunks +28 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (4 generated)
iannucci
5 years, 5 months ago (2015-07-24 00:32:05 UTC) #1
dnj (Google)
lgtm w/ nit https://codereview.chromium.org/1259463002/diff/1/service/rawdatastore/types.go File service/rawdatastore/types.go (right): https://codereview.chromium.org/1259463002/diff/1/service/rawdatastore/types.go#newcode61 service/rawdatastore/types.go:61: return "True" not: Perhaps lowercase "true"/"false", ...
5 years, 5 months ago (2015-07-24 16:11:37 UTC) #3
iannucci
Hm... Generally the string should represent the "repr" of the type, no? On Fri, Jul ...
5 years, 5 months ago (2015-07-24 16:15:06 UTC) #4
dnj (Google)
On 2015/07/24 16:15:06, iannucci wrote: > Hm... Generally the string should represent the "repr" of ...
5 years, 5 months ago (2015-07-24 16:17:34 UTC) #5
Vadim Sh.
https://codereview.chromium.org/1259463002/diff/20001/service/rawdatastore/properties.go File service/rawdatastore/properties.go (right): https://codereview.chromium.org/1259463002/diff/20001/service/rawdatastore/properties.go#newcode360 service/rawdatastore/properties.go:360: // TFlag BoolFlag `gae:"$flag1,true"` // defaults to true update ...
5 years, 5 months ago (2015-07-24 18:53:00 UTC) #6
iannucci
If you set a Toggle field, you MUST include a default, and it MUST be ...
5 years, 5 months ago (2015-07-24 19:06:15 UTC) #7
iannucci
PTAL https://chromiumcodereview.appspot.com/1259463002/diff/20001/service/rawdatastore/properties.go File service/rawdatastore/properties.go (right): https://chromiumcodereview.appspot.com/1259463002/diff/20001/service/rawdatastore/properties.go#newcode360 service/rawdatastore/properties.go:360: // TFlag BoolFlag `gae:"$flag1,true"` // defaults to true ...
5 years, 5 months ago (2015-07-24 20:58:10 UTC) #8
Vadim Sh.
lgtm
5 years, 5 months ago (2015-07-24 20:59:54 UTC) #9
commit-bot: I haz the power
Commit queue rejected this change because it did not recognize the base URL. Please commit ...
5 years, 5 months ago (2015-07-24 21:05:02 UTC) #13
Vadim Sh.
On 2015/07/24 21:05:02, commit-bot: I haz the power wrote: > Commit queue rejected this change ...
5 years, 5 months ago (2015-07-24 21:07:32 UTC) #14
iannucci
Oh. Rite. On Fri, Jul 24, 2015 at 2:07 PM <vadimsh@chromium.org> wrote: > On 2015/07/24 ...
5 years, 5 months ago (2015-07-24 21:10:02 UTC) #15
iannucci
5 years, 5 months ago (2015-07-24 21:10:05 UTC) #16
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as
92ee98fa28f06f5bdb297d6974ca085e566effff (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698