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

refactor: Remove the weak event managers #2276

Merged
merged 5 commits into from Dec 10, 2019
Merged

refactor: Remove the weak event managers #2276

merged 5 commits into from Dec 10, 2019

Conversation

@glennawatson
Copy link
Contributor

glennawatson commented Dec 9, 2019

This removes the weak event manager from ReactiveUI.

The weak event manager was originally introduced to attempt to solve memory leak issues with the WPF platform. We ended up getting WhenActivated from that instead.

At the moment it's working for every platform that doesn't actually require it.

Weak events generally are a anti-pattern with observables since you have explicit cleanup mechanisms with Observables and you should be using those instead.

@glennawatson glennawatson requested review from reactiveui/android-team as code owners Dec 9, 2019
glennawatson added 2 commits Dec 9, 2019
@glennawatson glennawatson requested a review from reactiveui/fody-team as a code owner Dec 9, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 9, 2019

Codecov Report

Merging #2276 into master will decrease coverage by 0.44%.
The diff coverage is 20%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2276      +/-   ##
==========================================
- Coverage   55.77%   55.32%   -0.45%     
==========================================
  Files         114      113       -1     
  Lines        4486     4334     -152     
  Branches      688      660      -28     
==========================================
- Hits         2502     2398     -104     
+ Misses       1803     1771      -32     
+ Partials      181      165      -16
Impacted Files Coverage Δ
src/ReactiveUI/ReactiveObject/ReactiveObject.cs 84.61% <ø> (+0.4%) ⬆️
...UI/Platforms/windows-common/ReactiveUserControl.cs 0% <ø> (ø) ⬆️
src/ReactiveUI.Winforms/ViewModelViewHost.cs 90.62% <0%> (-0.38%) ⬇️
src/ReactiveUI.Winforms/RoutedViewHost.cs 89.85% <0%> (-0.56%) ⬇️
src/ReactiveUI/Expression/Reflection.cs 60.27% <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 f0aefc1...286ae6b. Read the comment docs.

fix
fix
@glennawatson glennawatson requested review from reactiveui/uwp-team as code owners Dec 10, 2019
@glennawatson glennawatson merged commit 125387d into master Dec 10, 2019
2 of 4 checks passed
2 of 4 checks passed
codecov/patch 20% of diff hit (target 55.77%)
Details
codecov/project 55.32% (-0.45%) compared to f0aefc1
Details
ReactiveUI-CI #10.5.49+3ff02c166b succeeded
Details
license/cla All CLA requirements met.
Details
@glennawatson glennawatson deleted the remove-weak-events branch Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.