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

Fixes awaits #15

Merged
merged 4 commits into from
Jul 6, 2021
Merged

Conversation

Ascenio
Copy link
Contributor

@Ascenio Ascenio commented Jul 1, 2021

Fix/awaits for futures

Closes #14.

I've added the unawaited_futures to avoid regressions, but let me know if you disagree with it.

Release Notes

  • Fixes futures not being returned

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • The GitHub Actions pass building and linting. Linter returns no warnings or errors.
  • The QA checklist below has been completed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

It doesn't return Futures and so doesn't await for them.

Issue Number: #14

What is the new behavior?

Now it returns Futures when needed but also awaits for them.

Does this introduce a breaking change?

  • Yes
  • No

QA Checklist

UIKit Update Checklist (Minor or Patch Release)

  • Using the latest version of Agora's Video SDK
  • Example apps are all functional
  • Core features are still working (both ways across platforms)
    • Camera + Mic muting works for local and remote users
    • Users are added and removed correctly when they join and leave the channel
    • Older versions of the library gracefully handle changes (Create issue if not)
    • Builtin buttons all work as expected
  • Any newly deprecated methods are flagged as such inline and in documentation

UIKit Update Checklist (Major Release)

  • The above checklist is completed (except backwards compatibility)
  • Thoroughly tested for crashes, across multiple platforms at the same time

@Ascenio Ascenio changed the title Fix/awaits for futures Fixes awaits Jul 2, 2021
Copy link
Contributor

@tadaspetra tadaspetra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@tadaspetra tadaspetra changed the base branch from main to dev July 2, 2021 18:55
@Ascenio
Copy link
Contributor Author

Ascenio commented Jul 2, 2021

Oh sorry, I didn't noticed there was a dev branch.

@tadaspetra
Copy link
Contributor

Oh sorry, I didn't noticed there was a dev branch.

No worries, we didn't realize either. We have a meeting planned after this weekend to go through all the PR's and set up the GitHub even nicer.

As a side note, these are really great PR's and we really appreciate them 😊

@Ascenio
Copy link
Contributor Author

Ascenio commented Jul 2, 2021

I'm glad I could help. It's always nice to contribute to open source 😄

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.

[PROPOSAL] Make void functions return Future
3 participants