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

fix IsExecuting and Can Execute threading issue #2835

Merged
merged 3 commits into from Jul 24, 2021

Conversation

ChrisPulman
Copy link
Member

What kind of change does this PR introduce?

fix #2834

What is the current behaviour?

Executing fails if a TaskPool thread is selected

What is the new behaviour?

Executing works if a TaskPool thread is selected

What might this PR break?

none expected

Please check if the PR fulfils these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

Changed location of ObserveOn as found route cause of the fault.
@codecov
Copy link

codecov bot commented Jul 24, 2021

Codecov Report

Merging #2835 (b849692) into main (aaacea8) will increase coverage by 0.30%.
The diff coverage is 81.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2835      +/-   ##
==========================================
+ Coverage   71.50%   71.81%   +0.30%     
==========================================
  Files         137      137              
  Lines        4804     4828      +24     
==========================================
+ Hits         3435     3467      +32     
+ Misses       1369     1361       -8     
Impacted Files Coverage Δ
.../ReactiveUI/ReactiveCommand/ReactiveCommandBase.cs 100.00% <ø> (ø)
src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs 82.07% <80.48%> (+1.24%) ⬆️
...ctiveUI/ReactiveCommand/CombinedReactiveCommand.cs 81.35% <87.50%> (-0.13%) ⬇️
src/ReactiveUI/RxApp.cs 75.34% <0.00%> (+2.73%) ⬆️
...veUI/Platforms/net5/ComponentModelTypeConverter.cs 88.88% <0.00%> (+4.44%) ⬆️
...atforms/netcoreapp3/ComponentModelTypeConverter.cs 88.88% <0.00%> (+4.44%) ⬆️
...veUI/Platforms/net4/ComponentModelTypeConverter.cs 86.11% <0.00%> (+5.55%) ⬆️
...UI/Platforms/windows-common/ReactiveUserControl.cs 90.90% <0.00%> (+18.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aaacea8...b849692. Read the comment docs.

Copy link
Contributor

@glennawatson glennawatson left a comment

Choose a reason for hiding this comment

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

Change to be configurable

users should recheck their code to ensure correct operation and set the canExecuteScheduler as required.
@glennawatson glennawatson merged commit d68dca1 into main Jul 24, 2021
@glennawatson glennawatson deleted the fix_Executing_IsExecuting_FailingWhenOnTaskPool branch July 24, 2021 23:08
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ReactiveCommand.IsExecuting does not work when outputScheduler is RxApp.TaskpoolScheduler
2 participants