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

Make activity resolution payload methods return a reference #819

Closed
wants to merge 1 commit into from

Conversation

lucperkins
Copy link
Contributor

@lucperkins lucperkins commented Sep 20, 2024

What was changed

Changes the interface provided in #814 to return a Payload reference, as suggested in this Slack message.

Why?

Provides slightly more informative error messages and makes the methods no longer require ownership of ActivityResolution, which is generally an ergonomic win in Rust.

Checklist

  1. Closes no issues (it's a small change so I opted not to create an issue first)

  2. How was this tested: the standard battery of CI tests

  3. Any docs updates needed? I don't believe so

@lucperkins lucperkins requested a review from a team as a code owner September 20, 2024 10:30
@lucperkins
Copy link
Contributor Author

On second though, I don't love this because it makes an expression like this no longer compile:

let payload = ctx.activity(activity_options).await.success_payload_or_error();

You can make this a two-liner:

let res = ctx.activity(activity).await;
let payload = res.unwrap_ok_payload();

But that feels like an ergonomic downgrade to me, so I'm going to close this in favor of the status quo.

@lucperkins lucperkins closed this Sep 20, 2024
@lucperkins lucperkins deleted the success-payload-ref branch September 20, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant