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

Updated to make Rendering and FileSystem operations async by default. #354

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mwadams
Copy link

@mwadams mwadams commented Nov 11, 2019

This is a WIP PR that is not really intended to be merged, but I am creating it for discussion. (Hence it is created as a draft.)

It is a significant breaking change that changes the file system and rendering operations to be async-by-default.

I would like to create some side-by-side benchmarking to understand the perf impact of these changes in various scenarios.

@daviburg
Copy link
Member

Having async as an option, great. Forcing everything to async as a breaking change... maybe not great.

@mwadams
Copy link
Author

mwadams commented Feb 28, 2020

I entirely agree!

@microalps
Copy link
Contributor

As this seems to have forked to another library and no feedback on how we can make this backwards compatible, can we close this PR?

Copy link

@OpenSpacesAndPlaces OpenSpacesAndPlaces left a comment

Choose a reason for hiding this comment

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

Is there any reason that adding async/await is being held up? Seems like a good upgrade - especially for heavily modularized rendering.

@daviburg
Copy link
Member

daviburg commented Sep 2, 2022

Is there any reason that adding async/await is being held up? Seems like a good upgrade - especially for heavily modularized rendering.

This is held up because there is no contributor addressing the acknowledge issues with the draft PR. The draft was done in a major breaking way. If a contributor wanted to step up, the change to asynchronous would need to be introduced as a set of parallel async public APIs, so the existing sync public APIs are not altered, and no existing client is broken.

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.

None yet

4 participants