Skip to content

feature gate async - #1840

Open
drac-dot-gay wants to merge 2 commits into
linebender:mainfrom
drac-dot-gay:main
Open

feature gate async#1840
drac-dot-gay wants to merge 2 commits into
linebender:mainfrom
drac-dot-gay:main

Conversation

@drac-dot-gay

Copy link
Copy Markdown

this pr explores feature gating everything related to async execution, which is a feature i don't personally need for my usecase and potentially simplifies writing a baseview integration. there's more to be explored here, like allowing for a runtime besides tokio (see: #1833)

the way i did this is probably very messy and i would prefer to have a second pair of eyes on it

*staples "that was easy" sound*
i think we could make the runtime just a little more generic and not
pull in tokio (smol my beloved) but this is a good starting point for
discussion
@PoignardAzur

Copy link
Copy Markdown
Collaborator

I'm on vacation right now, will review this next week.

At a glance, I think we should go for a solution that uses fewer cfg blocks? I don't think xilem_core should get feature flags, for instance.

@drac-dot-gay

Copy link
Copy Markdown
Author

that's a good point, i was kind of overzealous at the start because i wanted to identify everything related to async, which also includes message proxies. i do also agree that a solution with less cfg blocks would be nice, but that would include some hefty refactoring. the approach that i would go for is wrap up all the async related functionality into one type that's generic over executors, so we're just gating the existence of one field/argument in structs and functions (and the type of course)

i'm also not sure how we want to handle the async action enum variant i gated out. i'd argue that we either keep it the way i did it or find some other way which doesn't involve that enum. either solution is fine, but it's just up to preference/philosophy i guess.

@PoignardAzur

Copy link
Copy Markdown
Collaborator

There's a good chance I just do the hefty refactoring and it supersedes your PR, for what it's worth.

If we're still not there in a month, then I'll probably merge this.

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