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 stop oaph omitting inappropriately #3693

Merged
merged 3 commits into from Dec 16, 2023
Merged

Fix stop oaph omitting inappropriately #3693

merged 3 commits into from Dec 16, 2023

Conversation

AmrAlSayed0
Copy link
Contributor

@AmrAlSayed0 AmrAlSayed0 commented Dec 7, 2023

Fixes #3682

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

For deferred subscriptions in OAPH 2 changes where made:

  • On first Value access INPC is no longer emitted while the Value is still the initial value.

  • After accessing Value for the first time, INPC is no longer emitted if the value produced from source is the same as the initial value

What might this PR break?
I don't think anything will break. It doesn't make sense for any UI framework to subscribe to INPC without reading the initial value anyway.

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)

Other information:

For deferred subscriptions in OAPH 2 changes where made:

- On first `Value` access INPC is no longer emitted while the `Value` is still the initial value.

- Before accessing `Value` for the first, INPC is no longer emitted if the value produced from source is the same as the initial value
@glennawatson
Copy link
Contributor

Might be worth also adding some unit tests. You basically made them anyway in your original issue you made.

@AmrAlSayed0
Copy link
Contributor Author

@glennawatson Done!

Copy link

codecov bot commented Dec 10, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (65377aa) 58.38% compared to head (63bdb8b) 58.37%.

Files Patch % Lines
...bservableForProperty/ObservableAsPropertyHelper.cs 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3693      +/-   ##
==========================================
- Coverage   58.38%   58.37%   -0.01%     
==========================================
  Files         160      160              
  Lines        5774     5776       +2     
  Branches     1028     1029       +1     
==========================================
+ Hits         3371     3372       +1     
  Misses       2015     2015              
- Partials      388      389       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glennawatson glennawatson changed the title Fixes #3682 Fix stop oaph omitting inappropriately Dec 10, 2023
@glennawatson glennawatson merged commit 0db5167 into reactiveui:main Dec 16, 2023
2 of 3 checks passed
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 Dec 31, 2023
@AmrAlSayed0 AmrAlSayed0 deleted the patch-1 branch December 31, 2023 09:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants