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

Introduce Mavericks core module #635

Merged
merged 15 commits into from
Jul 25, 2022
Merged

Conversation

sav007
Copy link
Collaborator

@sav007 sav007 commented Jun 13, 2022

Move everything that could be Android platform agnostic into the core module.

Part of #632

Move everything that could be Android platform agnostic into the core module.

Part of airbnb#632
@sav007 sav007 marked this pull request as draft June 13, 2022 19:02
sav007 added 3 commits June 19, 2022 07:37
Introduce `ExperimentalMavericksApi` annotation to repository declarations, to denote that repository is experimental API.

Extract `MavericksBlockExecutions` out from `MavericksViewModelConfig`, as `MavericksViewModelConfig` resides in `mvrx` module.

Replace `MavericksRepositoryConfigFactory` with `MavericksRepositoryConfigProvider` for simplicity.

Fix docs in `mvrx-core` module
@sav007 sav007 marked this pull request as ready for review June 24, 2022 03:03
@sav007 sav007 changed the title [WIP] Introduce Mavericks core module Introduce Mavericks core module Jun 24, 2022
Copy link
Contributor

@elihart elihart left a comment

Choose a reason for hiding this comment

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

Looking really good! mainly just have one question

@@ -30,6 +25,16 @@ fun assertMavericksDataClassImmutability(
) {
require(kClass.java.isData) { "Mavericks state must be a data class! - ${kClass.simpleName}" }

val disallowedFieldCollectionTypes = listOfNotNull(
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like a good solution 👍

Return back `MavericksViewModel.viewModelScope` for backward compatibility.
@sav007 sav007 requested a review from elihart June 28, 2022 13:55
@sav007 sav007 requested a review from gpeal July 2, 2022 03:09
@sav007 sav007 force-pushed the is/mavericks-core branch from 7566758 to ae78fc0 Compare July 2, 2022 04:16
Copy link
Contributor

@elihart elihart left a comment

Choose a reason for hiding this comment

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

looks great to me, thanks for contributing this improvement!

@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #635 (afc5b4e) into main (f44e85b) will decrease coverage by 7.24%.
The diff coverage is 18.38%.

@@            Coverage Diff             @@
##             main     #635      +/-   ##
==========================================
- Coverage   55.06%   47.81%   -7.25%     
==========================================
  Files          53       49       -4     
  Lines        2508     2704     +196     
  Branches      326      289      -37     
==========================================
- Hits         1381     1293      -88     
- Misses        976     1279     +303     
+ Partials      151      132      -19     
Impacted Files Coverage Δ
.../airbnb/mvrx/compose/MavericksComposeExtensions.kt 0.00% <ø> (ø)
.../airbnb/mvrx/hilt/HiltMavericksViewModelFactory.kt 18.18% <0.00%> (ø)
...main/kotlin/com/airbnb/mvrx/mocking/MockBuilder.kt 22.01% <0.00%> (-11.73%) ⬇️
...c/main/kotlin/com/airbnb/mvrx/BaseMvRxViewModel.kt 71.53% <0.00%> (ø)
mvrx/src/main/kotlin/com/airbnb/mvrx/Mavericks.kt 66.66% <ø> (+46.66%) ⬆️
...main/kotlin/com/airbnb/mvrx/MavericksExtensions.kt 8.18% <0.00%> (ø)
.../kotlin/com/airbnb/mvrx/ViewModelStateContainer.kt 20.00% <20.00%> (ø)
.../main/kotlin/com/airbnb/mvrx/MavericksViewModel.kt 62.02% <68.08%> (+1.31%) ⬆️
...tlin/com/airbnb/mvrx/MavericksViewModelProvider.kt 90.47% <80.00%> (+1.76%) ⬆️
...tlin/com/airbnb/mvrx/mocking/KotlinReflectUtils.kt 66.66% <81.25%> (+8.04%) ⬆️
... and 14 more

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 ed755e3...afc5b4e. Read the comment docs.

@sav007 sav007 requested a review from elihart July 14, 2022 04:07
Copy link
Contributor

@elihart elihart left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@sav007
Copy link
Collaborator Author

sav007 commented Jul 21, 2022

@elihart fixed lint issue

@elihart elihart merged commit 0ef2a9f into airbnb:main Jul 25, 2022
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.

3 participants