dataset.get's example code looks... broken?

[jgeewax]

Looking at http://googlecloudplatform.github.io/gcloud-node/#/docs/v0.23.0/datastore/dataset?method=get

I see

Where you see transaction, assume this is the context that's relevant to your use, whether that be a Dataset or Transaction object.

(empty area that looks like it should have code)

Get a single entity.

var key = dataset.key(['Company', 123]);

transaction.get(key, function(err, entity) {});

I think there are a few issues

1. The empty space looks like it should have code
2. The second snippet about getting a single entity refers to a variable that doesn't appear to be defined

Any idea what's up here?

[callmehiphop]

The empty space looks like it should have code

Looks like this is just a formatting error, at some point Get a single entity. was actually written in as a JS comment, it looks like when it was changed it created the empty code block.

http://googlecloudplatform.github.io/gcloud-node/#/docs/v0.14.0/datastore/dataset

The second snippet about getting a single entity refers to a variable that doesn't appear to be defined

I believe the intro sentence attempts to explain where transaction came from..

Where you see transaction, assume this is the context that's relevant to your use, whether that be a Dataset or Transaction object.

Would you prefer to remove that sentence and just create the transaction variable within the example?

[jgeewax]

Would you prefer to remove that sentence and just create the transaction variable within the example?

Yea -- I suspect people will be lazy (like I just was) and read the code carefully, but not necessarily the surrounding documentation.

I'd even be OK with something like:

transaction = /* Read above about how to get the transaction */ null;

But obviously working code would be better (particularly if it's just one line).

The goal of testing our snippets was copy/paste shouldn't throw syntax errors... so having an undefined variable like transaction makes me think "whoops they probably made a mistake" not "hmm they probably explained that in some text above this". Does that make sense?

[callmehiphop]

It totally makes sense. There actually is a transaction variable, but for whatever reason it's in the developer's documentation section.. so it never makes it to the site.

https://github.com/GoogleCloudPlatform/gcloud-node/blob/master/lib/datastore/request.js#L92

[callmehiphop]

Ah, so that particular file has a lot of references to transaction scattered throughout and we can only create transaction objects via callbacks, I'm assuming it was done this way to simplify the example. What are your feelings on something like..

function getCompany(transaction, done) {
  var key = dataset.key(['Company', 123]);

  transaction.get(key, function(err, entity) {
    done();
  });
}

dataset.runInTransaction(getCompany, function(err) {});

[jgeewax]

Hmm.. Maybe it's worth noting that when I show up on the datastore.get() docs page, I'm expecting something like..

var key = dataset.key(['Company', 1234]);
dataset.get(key, function(err, entity) {  // is this even valid?
  // .. Do something with your entity :)
});

That is -- I'm not really interested in transactions right now... I just want to know "how do I retrieve a key from this service?"

Since the docs are about dataset objects, I can assume that I already know how to get a dataset (if not, I go to the top and read the example there that shows how to grab a dataset).

[jgeewax]

Any update here ? Just went to look up the docs, saw the bug, went to master, saw the bug :(

[callmehiphop]

Pushed fixes for the bugs into #903.

As for the transaction vs dataset stuff, both are referencing the DatastoreRequest documentation, I can change it to dataset but then the transaction documentation will be wrong. A quick fix would be to add a second set of examples for dataset, does that sound ok?

[callmehiphop]

We currently do something similar for ACLs - http://googlecloudplatform.github.io/gcloud-node/#/docs/v0.24.0/storage/bucket?method=acl.add

[stephenplusplus]

Sgtm