Allow metadata fields to be PropertyConverters for symmetry.

Allow metadata fields to be PropertyConverters for symmetry.

This makes the easiest forms of parsed kinds/ids extremely simple to implement,
as well as composable. The MetaGetterSetter interface requires every top-level
struct to implement it, since allowing embedded implementations will get the
wrong value for the derived "$kind".

R=dnj@chromium.org
BUG=

Committed: https://github.com/luci/gae/commit/086748cd03c7bfc319eac9ab266764a219a68f37

[dnj, 2015-11-03 01:18:10]

Looks reasonable.

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
File service/datastore/pls_impl.go (right):

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:317: if err == nil {
nit: Handle err != nil (more standard).

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:337: val, err := p.getMetaFor(idx)
Do we care about non-nil errors? If not, have getMetaFor return a boolean
instead of an error.

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:470: if
reflect.PtrTo(ft).Implements(typeOfPropertyConverter) {
nit: st.convert = reflect.PtrTo(ft)....

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:486: mv, err := convertMeta(opts, f.Type)
nit: f.Type => ft

[iannucci, 2015-11-03 01:29:13]

ptal

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
File service/datastore/pls_impl.go (right):

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:337: val, err := p.getMetaFor(idx)
On 2015/11/03 at 01:18:10, dnj wrote:
> Do we care about non-nil errors? If not, have getMetaFor return a boolean
instead of an error.

eh, returning an error is standarder

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:470: if
reflect.PtrTo(ft).Implements(typeOfPropertyConverter) {
On 2015/11/03 at 01:18:10, dnj wrote:
> nit: st.convert = reflect.PtrTo(ft)....

DERP

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:486: mv, err := convertMeta(opts, f.Type)
On 2015/11/03 at 01:18:10, dnj wrote:
> nit: f.Type => ft

done (and others)

[iannucci, 2015-11-03 02:10:55]

sigh... ptal now that I actually uploaded the patchset :p

[dnj, 2015-11-03 03:11:25]

lgtm

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
File service/datastore/pls_impl.go (right):

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:337: val, err := p.getMetaFor(idx)
On 2015/11/03 01:29:12, iannucci wrote:
> On 2015/11/03 at 01:18:10, dnj wrote:
> > Do we care about non-nil errors? If not, have getMetaFor return a boolean
> instead of an error.
> 
> eh, returning an error is standarder

Gah I I don't like the feel of returning an error and then not responding to it.
I do think a bool is more appropriate here. I'll leave it up to you though; I
don't care that much.

https://chromiumcodereview.appspot.com/1414043006/diff/30001/service/datastor...
File service/datastore/pls.go (right):

https://chromiumcodereview.appspot.com/1414043006/diff/30001/service/datastor...
service/datastore/pls.go:135: // "Doe"}}`, retaining Jane's name as manipulable
go fields. However, in the
nit: capitalize Go.

https://chromiumcodereview.appspot.com/1414043006/diff/30001/service/datastor...
service/datastore/pls.go:137: // struct from the datastore as part of a Query,
for example, would correct
nit: correctly*

[iannucci, 2015-11-03 20:28:55]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2015-11-03 20:28:56]

The patchset sent to the CQ was uploaded after l-g-t-m from dnj@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1414043006/#ps50001
(title: "fix nits")

[commit-bot: I haz the power, 2015-11-03 20:29:03]

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

[iannucci, 2015-11-03 20:29:33]

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
File service/datastore/pls_impl.go (right):

https://chromiumcodereview.appspot.com/1414043006/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:337: val, err := p.getMetaFor(idx)
On 2015/11/03 at 03:11:25, dnj wrote:
> On 2015/11/03 01:29:12, iannucci wrote:
> > On 2015/11/03 at 01:18:10, dnj wrote:
> > > Do we care about non-nil errors? If not, have getMetaFor return a boolean
> > instead of an error.
> > 
> > eh, returning an error is standarder
> 
> Gah I I don't like the feel of returning an error and then not responding to
it. I do think a bool is more appropriate here. I'll leave it up to you though;
I don't care that much.

done

https://chromiumcodereview.appspot.com/1414043006/diff/30001/service/datastor...
File service/datastore/pls.go (right):

https://chromiumcodereview.appspot.com/1414043006/diff/30001/service/datastor...
service/datastore/pls.go:135: // "Doe"}}`, retaining Jane's name as manipulable
go fields. However, in the
On 2015/11/03 at 03:11:25, dnj wrote:
> nit: capitalize Go.

done

https://chromiumcodereview.appspot.com/1414043006/diff/30001/service/datastor...
service/datastore/pls.go:137: // struct from the datastore as part of a Query,
for example, would correct
On 2015/11/03 at 03:11:25, dnj wrote:
> nit: correctly*

done

[commit-bot: I haz the power, 2015-11-03 20:30:35]

Committed patchset #4 (id:50001) as
https://github.com/luci/gae/commit/086748cd03c7bfc319eac9ab266764a219a68f37