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: Rethrow exception on RaiseAndSetIfChanged #2035

Merged
merged 5 commits into from May 21, 2019

Conversation

Projects
None yet
2 participants
@RLittlesII
Copy link
Member

commented May 17, 2019

What kind of change does this PR introduce?

Resolves: #2013

What is the current behavior?

Exceptions thrown on RaiseAndSetIfChanged are not being rethrown if there is no "ThrownExceptions" subscribed.

What is the new behavior?

Exceptions thrown on RaiseAndSetIfChanged are rethrown if there is no "ThrownExceptions" observable subscriptions.

What might this PR break?

Please check if the PR fulfills these requirements

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

@RLittlesII RLittlesII requested a review from reactiveui/core-team as a code owner May 17, 2019

@RLittlesII RLittlesII changed the title fix: Throw exception on RaiseAndSetIfChanged fix: Rethrow exception on RaiseAndSetIfChanged May 17, 2019

@codecov

This comment has been minimized.

Copy link

commented May 17, 2019

Codecov Report

Merging #2035 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2035   +/-   ##
======================================
  Coverage    60.2%   60.2%           
======================================
  Files         116     116           
  Lines        4453    4453           
  Branches      639     639           
======================================
  Hits         2681    2681           
  Misses       1600    1600           
  Partials      172     172
Impacted Files Coverage Δ
...tiveUI/ReactiveObject/IReactiveObjectExtensions.cs 100% <ø> (ø) ⬆️

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 fda60ac...b9265c3. Read the comment docs.

@RLittlesII

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

@glennawatson Seems that adding the code for this broke a preexisting unit test. Seems that fixing this is in direct violation with how we are marshaling exceptions. It may require additional conversation.

glennawatson added some commits May 21, 2019

@glennawatson glennawatson merged commit 179e19d into master May 21, 2019

6 checks passed

ReactiveUI-CI Build #9.15.11+d5322da7da succeeded
Details
ReactiveUI-CI (Mac) Mac succeeded
Details
ReactiveUI-CI (Windows) Windows succeeded
Details
codecov/patch Coverage not affected when comparing fda60ac...b9265c3
Details
codecov/project 60.2% remains the same compared to fda60ac
Details
license/cla All CLA requirements met.
Details

@delete-merged-branch delete-merged-branch bot deleted the gh2013 branch May 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.