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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1354343002: service/datastore: misc improvements in properties.go (Closed)

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

Description

service/datastore: misc improvements in properties.go R=iannucci@chromium.org BUG= Committed: https://github.com/luci/gae/commit/5d31f23a6ff65d25a3cd26b0390d9075b8a73d91

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -29 lines) Patch
M service/datastore/properties.go View 4 chunks +4 lines, -29 lines 2 comments Download
A service/datastore/propertytype_string.go View 1 chunk +16 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
nodir
PTAL
5 years, 3 months ago (2015-09-19 17:46:40 UTC) #1
iannucci
lgtm https://codereview.chromium.org/1354343002/diff/1/service/datastore/properties.go File service/datastore/properties.go (right): https://codereview.chromium.org/1354343002/diff/1/service/datastore/properties.go#newcode97 service/datastore/properties.go:97: //go:generate stringer -type=PropertyType is stringer automatically included with ...
5 years, 3 months ago (2015-09-19 18:23:26 UTC) #2
iannucci
dnj/vadimsh are we set up for stringer goodness?
5 years, 3 months ago (2015-09-19 19:05:16 UTC) #4
Vadim Sh.
stringer should be fine https://codereview.chromium.org/1354343002/diff/1/service/datastore/properties.go File service/datastore/properties.go (right): https://codereview.chromium.org/1354343002/diff/1/service/datastore/properties.go#newcode97 service/datastore/properties.go:97: //go:generate stringer -type=PropertyType On 2015/09/19 ...
5 years, 3 months ago (2015-09-19 20:03:59 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1354343002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1354343002/1
5 years, 3 months ago (2015-09-22 17:20:01 UTC) #7
commit-bot: I haz the power
5 years, 3 months ago (2015-09-22 17:21:31 UTC) #8
Message was sent while issue was closed. 
Committed patchset #1 (id:1) as
https://github.com/luci/gae/commit/5d31f23a6ff65d25a3cd26b0390d9075b8a73d91

Powered by Google App Engine
This is Rietveld 408576698