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

Inject RecycledViewPool by using Dagger2 #662

Merged
merged 2 commits into from Feb 10, 2018

Conversation

@chibatching
Copy link
Contributor

@chibatching chibatching commented Feb 10, 2018

Issue

  • no issue

Overview (Required)

  • Share RecycledViewPool by using Dagger module
    • ViewModel must not reference a view instance. It may cause memory leaking.

Links

Screenshot

Before After
import io.github.droidkaigi.confsched2018.presentation.MainActivity

@Module interface MainActivityBuilder {
@ContributesAndroidInjector(modules = [MainActivityModule::class])
@ContributesAndroidInjector(modules = [MainActivityModule::class, RecycledViewPoolModule::class])

This comment has been minimized.

@droidkaigibot

droidkaigibot Feb 10, 2018

⚠️ Exceeded max line length (100)

This comment has been minimized.

@eyedol

eyedol Mar 6, 2018
Contributor

This is unrelated to the PR but I'm curious what tool you're using for the automatic code reviews. Looking to implement something similar for my projects. Thanks.

@jmatsu
Copy link
Member

@jmatsu jmatsu commented Feb 10, 2018

👀

import dagger.Module
import dagger.Provides

// Share RecycledViewPool between content fragments of ViewPager.

This comment has been minimized.

@jmatsu
jmatsu approved these changes Feb 10, 2018
Copy link
Member

@jmatsu jmatsu left a comment

LGTM!

@jmatsu jmatsu merged commit 2e0ddf7 into DroidKaigi:master Feb 10, 2018
2 checks passed
2 checks passed
ci/circleci Your tests passed on CircleCI!
Details
danger/danger All green. Woo!
@chibatching chibatching deleted the chibatching:share_view_pool_via_daggaer branch Feb 10, 2018
@chibatching
Copy link
Contributor Author

@chibatching chibatching commented Feb 10, 2018

Thanks! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants