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(crashadapter-leak) : Clear reference of crashAdapter after fragment is destroyed #233

Merged
merged 6 commits into from
Apr 27, 2023

Conversation

Piyush7890
Copy link
Contributor

Clear adapter reference once the fragment is destroyed.
To do this, I have modified the FragmentViewBindingDelegate class to accommodate all types of variables and not just binding.

Fixes #207

Let me know If I should continue working on this PR, basically modify all the adapters to use 'autoCleared'

To do this, I have modified the FragmentViewBindingDelegate class to accomodate all types of variables and not just binding.
@CLAassistant
Copy link

CLAassistant commented Apr 25, 2023

CLA assistant check
All committers have signed the CLA.

@srtvprateek
Copy link
Member

looks good to me,

can you also explore if we can keep definition & declaration together (not mandatory though)
like
private var crashAdapter: BaseAdapter by autoClear { CrashesAdapter(onActionListener) }

@Piyush7890
Copy link
Contributor Author

Piyush7890 commented Apr 26, 2023

What do you think about this approach? @srtvprateek
I have tried to be as clean as possible

@srtvprateek
Copy link
Member

srtvprateek commented Apr 26, 2023

nice 🔥
approach looks good, can make the change for other adapters too 👍🏽
1 feedback, can we rename autoCleared to autoClearInitializer for better understanding

@piyushsjsu
Copy link

Made the changes!

@srtvprateek
Copy link
Member

@Piyush7890 run ./gradlew ktlintFormat detekt before commit

@Piyush7890
Copy link
Contributor Author

@srtvprateek my bad. Tasks ran successfully

@srtvprateek srtvprateek merged commit 0a63442 into androidPluto:develop Apr 27, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory Leak: CoordinatorLayout is leaking
4 participants