Make appengine connection explicitly owned by impl/prod.

Make appengine connection explicitly owned by impl/prod.

R=vadimsh@chromium.org, hinoka@chromium.org
BUG=

Committed: https://github.com/luci/gae/commit/3a50526e08194bd66487c7c03cccd2108c236468

[iannucci, 2015-12-02 23:46:07]

iannucci@chromium.org changed reviewers:
+ dnj@chromium.org

[iannucci, 2015-12-02 23:46:07]

+dnj too

[Vadim Sh., 2015-12-03 00:49:58]

lgtm with suggestion (not sure how it would look though)

https://codereview.chromium.org/1490243004/diff/20001/impl/prod/datastore_key.go
File impl/prod/datastore_key.go (right):

https://codereview.chromium.org/1490243004/diff/20001/impl/prod/datastore_key...
impl/prod/datastore_key.go:44: func dsF2R(aeCtx context.Context, k *ds.Key)
(*datastore.Key, error) {
how do you feel about adding a typedef for AE context to avoid accidental
misuse? e.g.

type aeContext context.Context

func dsF2R(e aeContext ...) { ... }

?

[iannucci, 2015-12-03 00:55:20]

On 2015/12/03 at 00:49:58, vadimsh wrote:
> lgtm with suggestion (not sure how it would look though)
> 
>
https://codereview.chromium.org/1490243004/diff/20001/impl/prod/datastore_key.go
> File impl/prod/datastore_key.go (right):
> 
>
https://codereview.chromium.org/1490243004/diff/20001/impl/prod/datastore_key...
> impl/prod/datastore_key.go:44: func dsF2R(aeCtx context.Context, k *ds.Key)
(*datastore.Key, error) {
> how do you feel about adding a typedef for AE context to avoid accidental
misuse? e.g.
> 
> type aeContext context.Context
> 
> func dsF2R(e aeContext ...) { ... }
> 
> ?

I considered it, but interface types are transparently compatible :/. It's
roughly equivalent to the naming convention at that point.

[iannucci, 2015-12-03 00:55:51]

On 2015/12/03 at 00:55:20, iannucci wrote:
> On 2015/12/03 at 00:49:58, vadimsh wrote:
> > lgtm with suggestion (not sure how it would look though)
> > 
> >
https://codereview.chromium.org/1490243004/diff/20001/impl/prod/datastore_key.go
> > File impl/prod/datastore_key.go (right):
> > 
> >
https://codereview.chromium.org/1490243004/diff/20001/impl/prod/datastore_key...
> > impl/prod/datastore_key.go:44: func dsF2R(aeCtx context.Context, k *ds.Key)
(*datastore.Key, error) {
> > how do you feel about adding a typedef for AE context to avoid accidental
misuse? e.g.
> > 
> > type aeContext context.Context
> > 
> > func dsF2R(e aeContext ...) { ... }
> > 
> > ?
> 
> I considered it, but interface types are transparently compatible :/. It's
roughly equivalent to the naming convention at that point.

(I could make it a struct, then it would be checked, but that seemed possibly
even uglier)

[Vadim Sh., 2015-12-03 00:57:21]

okay

[iannucci, 2015-12-03 01:00:48]

The CQ bit was checked by iannucci@chromium.org

[commit-bot: I haz the power, 2015-12-03 01:01:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1490243004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1490243004/20001

[iannucci, 2015-12-03 01:02:28]

The CQ bit was unchecked by iannucci@chromium.org

[iannucci, 2015-12-03 01:05:34]

Description was changed from

==========
Make appengine connection explicitly owned by impl/prod.

R=hinoka@chromium.org, vadimsh@chromium.org
BUG=
==========

to

==========
Make appengine connection explicitly owned by impl/prod.

R=vadimsh@chromium.org, hinoka@chromium.org
BUG=

Committed:
https://github.com/luci/gae/commit/3a50526e08194bd66487c7c03cccd2108c236468
==========

[iannucci, 2015-12-03 01:05:35]

Committed patchset #2 (id:20001) manually as
3a50526e08194bd66487c7c03cccd2108c236468.