Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2802523002:
gae cloud: Use with Datastore (Closed)
|
Created:
3 years, 8 months ago by Ryan Tseng Modified:
3 years, 8 months ago CC:
chromium-reviews, infra-reviews+luci-gae_chromium.org Target Ref:
refs/heads/master Project:
luci-gae Visibility:
Public. |
Descriptiongae cloud: Use with Datastore
BUG=
Review-Url: https://codereview.chromium.org/2802523002
Committed: https://github.com/luci/gae/commit/4e7c36ffa6c3a0e4904c3577ac80f8878aa78c28
Patch Set 1 #
Total comments: 6
Patch Set 2 : review #Messages
Total messages: 17 (13 generated)
The CQ bit was checked by hinoka@google.com 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: This issue passed the CQ dry run.
nodir@chromium.org changed reviewers: + nodir@chromium.org
lgtm https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go File impl/cloud/context.go (right): https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go#newco... impl/cloud/context.go:54: cds := cloudDatastore{ use UseWithDatastore here https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go#newco... impl/cloud/context.go:75: // UseWithDatastore just install a datastore implementation into the context. UseDatastore installs a cloud datastore implementation into the context https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go#newco... impl/cloud/context.go:76: func UseWithDatastore(c context.Context, client *datastore.Client) context.Context { two verbs? I think it should be either UseDatastore or WithDatastore. Other functions in this code use Use, so maybe UseDatastore
The CQ bit was checked by hinoka@google.com 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...
hinoka@chromium.org changed reviewers: + hinoka@chromium.org
https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go File impl/cloud/context.go (right): https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go#newco... impl/cloud/context.go:54: cds := cloudDatastore{ On 2017/04/05 22:10:51, nodir wrote: > use UseWithDatastore here Done. https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go#newco... impl/cloud/context.go:75: // UseWithDatastore just install a datastore implementation into the context. On 2017/04/05 22:10:51, nodir wrote: > UseDatastore installs a cloud datastore implementation into the context Done. https://codereview.chromium.org/2802523002/diff/1/impl/cloud/context.go#newco... impl/cloud/context.go:76: func UseWithDatastore(c context.Context, client *datastore.Client) context.Context { On 2017/04/05 22:10:51, nodir wrote: > two verbs? I think it should be either UseDatastore or WithDatastore. Other > functions in this code use Use, so maybe UseDatastore Done.
The CQ bit was unchecked by hinoka@chromium.org
The CQ bit was checked by hinoka@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nodir@chromium.org Link to the patchset: https://codereview.chromium.org/2802523002/#ps20001 (title: "review")
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": 20001, "attempt_start_ts": 1491500787331870,
"parent_rev": "272e08846966144869f38a9f9f4cc151082b4975", "commit_rev":
"4e7c36ffa6c3a0e4904c3577ac80f8878aa78c28"}
Message was sent while issue was closed.
Description was changed from ========== gae cloud: Use with Datastore BUG= ========== to ========== gae cloud: Use with Datastore BUG= Review-Url: https://codereview.chromium.org/2802523002 Committed: https://github.com/luci/gae/commit/4e7c36ffa6c3a0e4904c3577ac80f8878aa78c28 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://github.com/luci/gae/commit/4e7c36ffa6c3a0e4904c3577ac80f8878aa78c28 |
