refactor: Improve var name. b/131773884

[grant]

Improve var name.
See: b/131773884

[ace-n]

LGTM, but want to run this past @stew-r as well.

[stew-r]

Urgh. While I really want to do this I think we would cause more confusion than we would be solving. Today, IIRC, that object is not == PubsubMessage. It's only the data field from that object.

EDIT: scratch that. Need to do some additional investigation.

[grant]

Urgh. While I really want to do this I think we would cause more confusion than we would be solving. Today, IIRC, that object is not == PubsubMessage. It's only the data field from that object.

EDIT: scratch that. Need to do some additional investigation.

Are you suggesting that the data field in the PubsubMessage has a nested data field as in data.data?

This PR has no functional changes, just inlining a variable.

[stew-r]

I'm saying that if you write a function (subscribed to a Pub/Sub topic) like the following:

exports.helloPubSub = async (pubsubMessage, context) => {
  console.log(pubsubMessage);
}

And then publish a message to this Pub/Sub topic, you will get the following output:

{ 
  '@type': 'type.googleapis.com/google.pubsub.v1.PubsubMessage',
  attributes: { test_key: 'test_value' },
  data: 'dGVzdF9tZXNzYWdl' 
}

That's almost, but not quite, the same as PubsubMessage:

{
  "data": string,
  "attributes": {
    string: string,
    ...
  },
  "messageId": string,
  "publishTime": string
}

This is a point of confusion for users. We can talk about why this is the case and whether it should be changed.

[ace-n]

Thought: would `pubsubSnippet` (or `pubsubMsgSnippet`) be a better name for this?

…

On Mon, May 6, 2019 at 9:46 AM Stewart Reichling ***@***.***> wrote: I'm saying that if you write a function (subscribed to a Pub/Sub topic) like the following: exports.helloPubSub = async (pubsubMessage, context) => { console.log(pubsubMessage); } And then publish a message to this Pub/Sub topic, you will get the following output: { ***@***.***': 'type.googleapis.com/google.pubsub.v1.PubsubMessage', attributes: { test_key: 'test_value' }, data: 'dGVzdF9tZXNzYWdl' } That's almost, but not quite, the same as PubsubMessage <https://cloud.google.com/pubsub/docs/reference/rest/v1/PubsubMessage>: { "data": string, "attributes": { string: string, ... }, "messageId": string, "publishTime": string } This is a point of confusion for users. We can talk about why this is the case and whether it should be changed. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#1262 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARTZRMWTGH2Q4OCTRCAVL3PUBOHVANCNFSM4HKVTJLA> .

-- Ace Nassri | Developer Programs Engineer | Cloud Developer Relations | [email protected] *If I have unusual email/ping hours, I am either **taking flex-time or** sleeping off an illness. Please know that I don't expect (and may not be able to give) a response immediately.*

[grant]

This change doesn't introduce functional changes. It simply removes an extra const from the sample as suggested in the bug report i.e. read something less confusing.

It does not attempt to edit the parameter type or solve the confusion Stewart mentioned.

[ace-n]

Is there a reason we shouldn't resolve that confusion in this PR, if something like pubsubSnippet would do the trick?

[stew-r]

I'd be more comfortable with something like `pubsubEvent`. TBH I don't think this is a major point of confusion. Your call on what you want to call this variable.

…

On Mon, May 6, 2019 at 10:41 AM Ace Nassri ***@***.***> wrote: Is there any reason we *shouldn't* resolve that confusion in this PR, if something like pubsubSnippet would do the trick? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1262 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHYELQGUOPZHRPXGDRMHAL3PUBUWLANCNFSM4HKVTJLA> .

-- Stewart Reichling | Product Manager, Serverless | [email protected] | 650.906.3381

[grant]

As discussed via Hangouts with Stewart, pubsubData or pubsubEvent are both fine. Changed the PR to reflect that.
PTAL.

[ace-n]

LGTM. Thanks for following up @grant!