-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Return ScheduledMessageID everywhere #1046
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More than 4 or 5 return values for a public function/method is too many,
so we would like to consider some other approach.
@zchee What do you think? |
Isn't scheduled message ID applicable to SendScheduledMessage API only? And messageTs is applicable to only SendMessage API. IMO, this change makes it more confusing, and I personally don't like multi-values-return that returns more than 2 values because it's hard to know which one is what without a name. I'd replicate |
I proposed the fix for this in the least intrusive way here #1153 |
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
This PR was closed because it has been stalled for 10 days with no activity. |
Should this be an issue instead
API changes
This extends
SendMessage*
to returnscheduled_message_id
everywhere. It would be great if the impact of this change could be smaller, but the commonSendMessage*
functionality is used everywhere so needed to be extended everywhere it was used. If there's a better way to exposescheduled_message_id
in a more limited fashion, that would be great.Fixes #757