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

Apply flowOn(defaultDispatchers) for main-safe. #1238

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

Jaehwa-Noh
Copy link
Contributor

@Jaehwa-Noh Jaehwa-Noh commented Mar 2, 2024

What I have done and why
I've added withContext(defaultDispatcher) for main-safe at combine.

Fix #1420

DefaultSearchContentsRepository.searchContents

Before After
image image

DefaultSearchContentsRepository.getSearchContentsCount

Before After
image image

GetFollowableTopicsUseCase.invoke

Before After
image image

Flow.mapToUserSearchResult

Before After
image image

Change-Id: Ic3474b86f1af0aaf80c13769223409d7ba070871
Change-Id: Ibadc91344a9ac57fed8449e5174a41e25ec7271e
Change-Id: I3bd11f32b42cb224f4de1b58616c52e0fd982833
Change-Id: I0e050bcecff5d43050103a9a19d0d6b6b31d4ba2
Change-Id: I142352cd418b147e538e8982b4b4dc17fb6b0d53
Copy link
Contributor

@hoc081098 hoc081098 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about

combine(a, b) { ... }
    .flowOn(defaultDispatcher)

I think the above code has the same meaning.

image

@Jaehwa-Noh
Copy link
Contributor Author

combine(a, b) { ... }
    .flowOn(defaultDispatcher)

Good suggestion. I'll accept your suggestion.
Thank you.

@Jaehwa-Noh Jaehwa-Noh marked this pull request as draft March 4, 2024 09:53
Change-Id: Iec1e30326807f79753ec68ee10f2ef5f4ad40535
Change-Id: I679f11aa0bcae1d2de54b0373a284f83d0f0c81b
@Jaehwa-Noh Jaehwa-Noh marked this pull request as ready for review March 4, 2024 12:13
- Style.

Change-Id: Iefae14197b1347c7fb00c2d1f754d51cf5419b80
@dturner
Copy link
Collaborator

dturner commented Mar 5, 2024

Getting a build error:

e: file:///home/runner/work/nowinandroid/nowinandroid/core/data/src/test/kotlin/com/google/samples/apps/nowinandroid/core/data/CompositeUserNewsResourceRepositoryTest.kt:40:9 No value passed for parameter 'defaultDispatcher'

@Jaehwa-Noh Jaehwa-Noh marked this pull request as draft March 6, 2024 06:37
Change-Id: Ie68dda7a4082d994e2d8d82c1737587f95ab15a0
Change-Id: I185e86f9481bfdb9dea014acb403f00a41edd434
Change-Id: Iee06da9ddded4ed5ba19bbfd5652b4a7a38f4cb9
Change-Id: Idb9fe56112f3764a1d20ae7499b8e9be37674188
Change-Id: I4db56855bb25aa8036b3a2ed5b9c5fd40d8468e1
Change-Id: I0da5c272dcda1355b573fa51746256309fde5d8a
Change-Id: Idff0551cc91d27df7a582f822df62f7b96b907ed
Change-Id: I343a85dfb6d9a101cd6934eceda803c0456184cf
@Jaehwa-Noh Jaehwa-Noh changed the title Apply withContext(defaultDispatchers) for main-safe. Apply flowOn(defaultDispatchers) for main-safe. Mar 8, 2024
Change-Id: Icf972886746a53662bae4e70f04f2499c8295236
@Jaehwa-Noh
Copy link
Contributor Author

#1250 This issue had given the clue that how to solve the test fail problems when I was struggling with some tests fail.
The key was single scheduler!

Thanks.

@Jaehwa-Noh Jaehwa-Noh marked this pull request as ready for review March 8, 2024 07:00
@Jaehwa-Noh Jaehwa-Noh marked this pull request as draft May 9, 2024 09:45
Change-Id: Iac1e30f357149fc4c6076d688088f6a54fce73da
Change-Id: I0939a10e2a8779844d59f9f07ef777ad94283811
@Jaehwa-Noh Jaehwa-Noh marked this pull request as ready for review May 9, 2024 11:18
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.

[Bug]: Some combine works on Main thread
3 participants