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

Update FS-1097-task-builder.md #571

Merged
merged 1 commit into from
May 26, 2021
Merged

Update FS-1097-task-builder.md #571

merged 1 commit into from
May 26, 2021

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented May 26, 2021

Update spec of backgroundTask { ... }, based on rspeele/TaskBuilder.fs#35 (comment)

@dsyme dsyme merged commit f371ae3 into master May 26, 2021
@NinoFloris
Copy link
Contributor

NinoFloris commented May 26, 2021

I think it shouldn't do this, backgroundTask screams 'everything inside of this will not run on this thread' yet a single while true at the start would now halt my thread. A context insenstive builder would serve as a better option if we want that mixed mode behavior IMO.

Even if we'd decide to keep this new semantics just checking SyncCtx is not enough. Tasks also respect TaskScheduler.Current meaning we must include another check to see if it's anything but TaskScheduler.Default.

@dsyme
Copy link
Contributor Author

dsyme commented May 26, 2021

@NinoFloris Thanks for this.

Let's discuss on the discussion thread

@smoothdeveloper
Copy link
Contributor

@dsyme, AFAIR, this is a split of a larger RFC, but it still points to the larger RFC and I think the semantics of task from end user point of view don't depend on the compiler machinery of the larger RFC.

Would it make sense to have a separate new discussion thread on just task CE?

@dsyme
Copy link
Contributor Author

dsyme commented May 26, 2021

Would it make sense to have a separate new discussion thread on just task CE?

It would

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.

3 participants