Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pubsub.Subscription.ack()'s docstring to say the ID is explicitly an ackId #518

Closed
jgeewax opened this issue May 1, 2015 · 3 comments · Fixed by #520
Closed

Update pubsub.Subscription.ack()'s docstring to say the ID is explicitly an ackId #518

jgeewax opened this issue May 1, 2015 · 3 comments · Fixed by #520
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Milestone

Comments

@jgeewax
Copy link
Contributor

jgeewax commented May 1, 2015

@pierre-b wrote in #487:

Could you please update the doc, the message.id should be message.ackId https://googlecloudplatform.github.io/gcloud-node/#/docs/v0.13.2/pubsub/subscription?method=ack

At first I thought we had this right, but it seems like the messageId might not be the same as the ackId (see RecievedMessage which says "ackId: This ID can be used to acknowledge the received message.")

Note: I still think we should call the parameter "ids", but that we should update the docstring.

On https://github.com/GoogleCloudPlatform/gcloud-node/blob/686fe62e7103f80dc053417e08c13abfc501f67f/lib/pubsub/subscription.js#L250

s/message IDs/message ackIds/
@jgeewax jgeewax added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. docs api: pubsub Issues related to the Pub/Sub API. labels May 1, 2015
@jgeewax jgeewax added this to the Pub/Sub Beta milestone May 1, 2015
@ryanseys
Copy link
Contributor

ryanseys commented May 1, 2015

Simple enough. I will change this today.

@jgeewax
Copy link
Contributor Author

jgeewax commented May 1, 2015

Style I was thinking was @param {string|string[]} ids - A single or multiple message acknowledgement IDs. but feel free to write whatever.

Just want to make it clear that a Message itself doesn't have an ID we care about. Instead a ReceivedMessage has an acknowledgement ID (ackId) which you pass around.

@ryanseys
Copy link
Contributor

ryanseys commented May 1, 2015

Yeah I'll come up with something. I understand the confusion.

sofisl pushed a commit that referenced this issue Oct 11, 2022
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/1c7a19e0-0642-4da9-a86e-676ae8965b89/targets

- [ ] To automatically regenerate this PR, check this box.

Source-Link: googleapis/synthtool@5451633
sofisl pushed a commit that referenced this issue Oct 13, 2022
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/1c7a19e0-0642-4da9-a86e-676ae8965b89/targets

- [ ] To automatically regenerate this PR, check this box.

Source-Link: googleapis/synthtool@5451633
sofisl pushed a commit that referenced this issue Nov 11, 2022
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/f20ba027-76a5-44ba-baa4-962e18a97819/targets

- [ ] To automatically regenerate this PR, check this box.
sofisl pushed a commit that referenced this issue Nov 11, 2022
sofisl pushed a commit that referenced this issue Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants