Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

bdero
Copy link
Member

@bdero bdero commented Sep 12, 2023

Add warning that suggests the workaround inline and points to flutter/flutter#134573.

Aside from this, we could also update our GLFW example embedder to employ this workaround (providing an explicit code sample that users may reference/copy).

@bdero bdero added affects: engine embedder Related to the embedder API labels Sep 12, 2023
@bdero bdero requested review from zanderso and gaaclarke September 12, 2023 20:18
@bdero bdero requested a review from cbracken as a code owner September 12, 2023 20:18
@bdero bdero self-assigned this Sep 12, 2023
@bdero bdero requested a review from chinmaygarde as a code owner September 12, 2023 20:18
@github-actions github-actions bot removed the embedder Related to the embedder API label Sep 12, 2023
Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

I'm already working on a PR that includes the unit test that does the swapping out so the docstring can reference it.

@bdero
Copy link
Member Author

bdero commented Sep 12, 2023

Does the docstring look good? This just adds a docstring and doesn't need a unittest. I'm OK to hold off though, no strong opinions here.

@gaaclarke
Copy link
Member

Does the docstring look good? This just adds a docstring and doesn't need a unittest. I'm OK to hold off though, no strong opinions here.

I'm minutes from posting the PR as we discussed in our meeting. I have something different written up that documents queue and the proc callback, plus it has the test that asserts it works.

@gaaclarke
Copy link
Member

Here is the PR with the test (#45732).

@bdero
Copy link
Member Author

bdero commented Sep 12, 2023

Closing in favor of #45732.

@bdero bdero closed this Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants