Maintenance: Centralized Value for Default IScheduler instance #862
Conversation
dwcullop
changed the title
src/DynamicData/Internal/Defaults.cs
Outdated
|
|
||
| namespace DynamicData.Internal; | ||
|
|
||
| internal static class Defaults |
There was a problem hiding this comment.
@RolandPheasant Do you think this should be a public thing so that consumers can change the value if they want to? We could call it DynamicData.GlobalConfig with this as a property or something.
It does sound like a foot-gun though. Maybe it should just be some enum that allows us to ensure it only gets set to schedulers that will work. Then again, maybe they have a reason for doing everything on the Intermediate Scheduler... Who can say?
There was a problem hiding this comment.
I suspect in this case, making it internal would probably be sufficient and it allows up to change our mind and give the consumers the best choice out of the box.
Also when I added the binding options I accidentally added the global options class in the binding namespace. That should be moved to the root namespace. It would technically be a breaking change, but I'd suggest we should not increase the bug version as it would be correcting a mistake for a bit of code for which there's probably only one or two users.
There was a problem hiding this comment.
See DynamicDataOptions here https://github.com/reactivemarbles/DynamicData/blob/main/src/DynamicData/Binding/BindingOptions.cs
There was a problem hiding this comment.
Funny you should ask... I just moved it to the main namespace.
Are you asking me to move the BindingOptions as part of this PR? They can't go in the same class if one is public and the other is internal.
There was a problem hiding this comment.
I moved it to the main namespace and gave it a better name but kept it as internal. If you want, I can move the DynamicDataOptions as well (but it might be better as another PR).
Then later we can discuss adding a configurable Default Scheduler value to it...
There was a problem hiding this comment.
@RolandPheasant See #864 for those changes.
…op/DynamicData into feature/default-scheduler
src/DynamicData/GlobalConfig.cs
Outdated
|
|
||
| internal static class GlobalConfig | ||
| { | ||
| public static IScheduler DefaultScheduler { get; } = TaskPoolScheduler.Default; |
There was a problem hiding this comment.
@RolandPheasant This is moved / renamed version.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Addresses #854 by defining a constant value for a default IScheduler instance and changes all instances of
Scheduler.Defaultto use that value instead.