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

Fix compiler warnings in Future implementation #168

Merged
merged 1 commit into from
May 17, 2024

Conversation

jb-gcx
Copy link
Contributor

@jb-gcx jb-gcx commented May 8, 2024

  • Unused parameters caused warnings in my project, so I removed them
  • I think Future.get() is intended to return a non-nullable - unless I'm missing some reason why it shouldn't?

@li-feng-sc
Copy link
Contributor

Hi, thanks for the PR. While the compiler warning fix is great, I'm not 100% sure about nonnull on the ObjC future.get method. I can see the "setValue" method also takes a "nullable", so in theory we may indeed get a null from get.

@jb-gcx
Copy link
Contributor Author

jb-gcx commented May 13, 2024

So if I understand correctly, in the ObjC version nil is always a possible result, regardless of the value type I use? I might argue that this should change, but I'll make that argument in a separate PR. I've removed the questionable commit from this PR 👍

Edit: I've created #169 for the nullability question.

@li-feng-sc li-feng-sc merged commit 47e3ddd into Snapchat:main May 17, 2024
1 check passed
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.

2 participants