Open
Conversation
WalkthroughComprehensive refactoring spanning ~90 files: rebranded repository references; converted mutable models to immutable; replaced manual Parcelable implementations with Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/build.gradle.kts`:
- Around line 36-38: The kotlin {} block requires the Kotlin Android plugin to
be applied; add alias(libs.plugins.jetbrainsKotlinAndroid) to the plugins block
alongside alias(libs.plugins.androidApplication) and
alias(libs.plugins.compose.compiler) so the org.jetbrains.kotlin.android plugin
is applied for the kotlin { jvmToolchain(17) } configuration, and ensure
gradle/libs.versions.toml defines the plugin alias as jetbrainsKotlinAndroid = {
id = "org.jetbrains.kotlin.android", version.ref = "kotlin" } so
libs.plugins.jetbrainsKotlinAndroid resolves.
In `@appupdater/src/main/java/com/pouyaheydari/appupdater/main/dsl/DSLUtils.kt`:
- Around line 13-55: Extract the duplicated builder classes StoreListItemBuilder
and DirectDownloadListItemBuilder into a shared module (e.g., :store or
:dsl-core) and replace the duplicate implementations in the Compose and Fragment
DSL modules with imports from that shared module; update the build system
(module dependency) and package/imports so UpdaterDialogDataBuilder continues to
reference the shared StoreListItemBuilder/DirectDownloadListItemBuilder types
and ensure the shared builders still produce StoreListItem and
DirectDownloadListItem with the same property names (store, title, icon, url)
and build() signatures.
- Around line 14-20: Replace the lateinit store property in StoreListItemBuilder
with a nullable var (store: AppStore? = null) and add explicit validation in
build(): check that store is not null and throw a clear IllegalStateException
(e.g., "StoreListItemBuilder.store must be set") before constructing
StoreListItem; update the build() method to use the validated non-null store
when calling StoreListItem(store = ..., title = title, icon = icon) so callers
get a helpful error instead of UninitializedPropertyAccessException.
In
`@appupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/AppUpdaterDialog.kt`:
- Around line 139-144: Remove the unused sealed variants
DialogScreenStates.Empty, DialogScreenStates.ShowUpdateInProgress, and
DialogScreenStates.HideUpdateInProgress from the sealed interface definition in
DialogScreenStates (delete their enum/variant declarations) and delete the dead
when-branch in AppUpdaterDialog.kt that matches those three cases (the branch
containing the comment "handled by screenState"). After removal, search for any
remaining references to those symbols (e.g., usages in other files or imports)
and clean them up, then run a build to ensure no unresolved references remain.
In
`@appupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/AppUpdaterViewModel.kt`:
- Around line 41-48: In AppUpdaterViewModel where DialogScreenIntents are mapped
to _sideEffect.trySend (for cases like DialogScreenIntents.OnDirectLinkClicked,
OnStoreClicked, OnOpeningStoreFailed producing DialogScreenStates.DownloadApk,
OpenStore, ExecuteErrorCallback), handle the trySend result: inspect the
ChannelResult (use onFailure or check isFailure) and either log the failure
(include a TAG and context such as which DialogScreenState failed) or switch to
calling send() from a coroutine scope for critical events (e.g., DownloadApk) so
failures are not dropped silently.
In
`@appupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/model/UpdaterDialogData.kt`:
- Around line 21-28: UpdaterDialogData was changed to immutable by turning its
public properties (title, description, storeList, directDownloadList,
isForceUpdate, typeface, errorWhileOpeningStoreCallback, theme) into vals, which
is a breaking API change for callers that mutate the instance; restore the
original mutability by making those properties vars again on the
UpdaterDialogData data class (or alternatively provide explicit copy/with or
builder APIs preserving binary compatibility) so existing consumers that set
properties after construction continue to work.
In `@compose/src/main/java/com/pouyaheydari/appupdater/compose/dsl/DSLUtils.kt`:
- Around line 13-19: The builder currently uses lateinit var store which causes
an UninitializedPropertyAccessException if unset; change store to a nullable var
store: AppStore? = null in StoreListItemBuilder and add an explicit validation
in build() (e.g., requireNotNull(store) { "StoreListItemBuilder.store must be
set before build()" }) before constructing and returning StoreListItem (use the
non-null value after validation). This provides a clear error message and avoids
lateinit.
- Around line 24-29: The builder DirectDownloadListItemBuilder currently allows
title and url to remain empty; update build() to validate required fields (title
and url) before constructing DirectDownloadListItem: check that title is not
blank and url is a non-empty/valid string (or matches a simple URL pattern) and
throw a clear IllegalStateException or IllegalArgumentException if validation
fails, so callers get an early, descriptive error instead of creating an invalid
DirectDownloadListItem.
In
`@directdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/data/DirectDownloadListItem.kt`:
- Around line 13-15: The data class DirectDownloadListItem changed its public
properties title and url from var to val which removes setters and is a breaking
API/ABI change; restore compatibility by making the properties mutable again
(use var for title and url on DirectDownloadListItem) so existing consumers and
generated setters remain available, ensuring the public API surface is
unchanged.
In
`@directdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/APKFileProvider.kt`:
- Line 1: The package rename changed the FQCN of the public interface
APKFileProvider and will break downstream imports; restore a temporary
compatibility shim by adding a deprecated typealias named APKFileProvider in the
old package that aliases the new APKFileProvider (so existing imports still
resolve), annotate it with `@Deprecated` with a message pointing to the new
package/FQCN and a ReplaceWith suggestion, and keep the shim for at least one
release before removal to give users time to migrate.
In
`@directdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/DownloadAPKHelper.kt`:
- Line 1: The package rename changed the FQN of checkPermissionsAndDownloadApk
and will break existing consumers; restore a deprecated compatibility wrapper in
the old package by adding a new file in the previous package namespace
(utils.donwloadapk) that declares a public function
checkPermissionsAndDownloadApk annotated `@Deprecated` which simply forwards its
parameters to the relocated implementation (the new
checkPermissionsAndDownloadApk) and re-exports any needed types, and include a
deprecation message pointing callers to the new package and note removal in the
next release.
In `@gradle/libs.versions.toml`:
- Around line 6-7: The agp version declared as agp = "9.1.0" in
libs.versions.toml is invalid; update the agp entry to a real released Android
Gradle Plugin version (for example "2.3.0" or the intended target) by replacing
the value for the agp key, then verify resolution against Maven Central and that
the Gradle wrapper (9.4.0) and Kotlin (kotlin = "2.3.20") remain compatible;
after changing the agp value, run a Gradle sync/build to ensure the plugin
resolves correctly.
In `@README.md`:
- Line 155: Update the two heading lines "Fragment DSL" and "Compose DSL" to use
one-level deeper headings (change from #### to ###) so they correctly follow the
parent "DSL Builders" (which is ##) and comply with MD001; locate the headings
by their exact text ("Fragment DSL" and "Compose DSL") in README.md and replace
the four-hash markers with three-hash markers.
- Line 2: The image tag(s) in README.md are missing alt text; update each <img>
element (e.g., the <img
src="https://raw.githubusercontent.com/HeyPouya/AndroidAppUpdater/master/pics/icon.png"
width="250">) to include a descriptive alt attribute (for example alt="Android
App Updater icon" or equivalent) to satisfy accessibility/MD045 requirements and
apply the same change to the other image(s).
- Around line 314-322: Update the fenced code block that starts with the line
"AndroidAppUpdater/" so the opening triple backticks include a language
identifier (e.g., change ``` to ```text) to satisfy MD040; locate the block
containing the ASCII architecture diagram (the lines showing
"AndroidAppUpdater/" and its subdirectories) and add the identifier to the
opening ``` only.
In
`@store/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreIntentBuilder.kt`:
- Around line 26-28: In Builder.withPackage (the function that sets
storePackageName) the require message has a typo "most" — update the exception
string to read "must not be empty" (i.e., change the require message for
storePackageName in withPackage to "Store's package name must not be empty") so
the validation message is correct; locate the require(...) call in
Builder.withPackage and replace the message accordingly.
In
`@store/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreListItem.kt`:
- Around line 15-18: The default for StoreListItem.icon was changed to 0 which
breaks previews/tests that relied on the previous drawable default; update any
preview/test instantiations of StoreListItem to pass an explicit drawable
resource (e.g., R.drawable.appupdater_ic_cloud or another appropriate drawable)
when constructing StoreListItem, or alternatively revert/change the default in
the StoreListItem declaration; search for usages of StoreListItem in
preview/test files and add the icon argument to each call to restore the
expected icon behavior.
In
`@store/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/NineApps.kt`:
- Around line 4-15: Add the Kotlin Parcelize plugin to every module that uses
`@Parcelize` (modules: store, directdownload, appupdater) so files like NineApps
(class NineApps in NineApps.kt) compile; update each module's Gradle settings to
apply the Kotlin parcelize plugin (the kotlin-parcelize /
org.jetbrains.kotlin.plugin.parcelize plugin) in the plugins block of that
module's build.gradle.kts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: aa7b2ed7-9ddd-4b2b-bdd4-5aad6589f9cb
📒 Files selected for processing (64)
README.mdapp/build.gradle.ktsappupdater/consumer-rules.proappupdater/src/main/java/com/pouyaheydari/appupdater/main/dsl/DSLUtils.ktappupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/AppUpdaterDialog.ktappupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/AppUpdaterViewModel.ktappupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/adapters/DirectRecyclerAdapter.ktappupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/adapters/StoresRecyclerAdapter.ktappupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/model/UpdaterDialogData.ktappupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/model/UpdaterFragmentModel.ktappupdater/src/main/java/com/pouyaheydari/appupdater/main/utils/ErrorCallbackHolder.ktappupdater/src/test/java/com/pouyaheydari/appupdater/main/ui/AppUpdaterViewModelTest.ktbuild-logic/convention/src/main/java/com/pouyaheydari/appupdater/convention/plugins/AndroidLibraryPlugin.ktbuild.gradle.ktscompose/consumer-rules.procompose/src/main/java/com/pouyaheydari/appupdater/compose/dsl/DSLUtils.ktcompose/src/main/java/com/pouyaheydari/appupdater/compose/ui/AndroidAppUpdaterScreen.ktcompose/src/main/java/com/pouyaheydari/appupdater/compose/ui/AndroidAppUpdaterViewModel.ktcompose/src/main/java/com/pouyaheydari/appupdater/compose/ui/models/UpdaterDialogData.ktdirectdownload/consumer-rules.prodirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/data/DirectDownloadListItem.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/domain/DownloadState.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/receiver/DownloadFinishedReceiver.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/APKDownloadManager.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/APKFileProvider.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/APKFileProviderImpl.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/DownloadAPKHelper.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/DownloadManagerRequestCreator.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/permission/DownloadAPKPermissionFactory.ktdirectdownload/src/main/kotlin/com/pouyaheydari/appupdater/directdownload/utils/permission/DownloadAPKPermissionForOAndBelow.ktdirectdownload/src/test/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/APKDownloadManagerTest.ktdirectdownload/src/test/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/APKFileProviderImplTest.ktdirectdownload/src/test/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/DownloadAPKHelperKtTest.ktdirectdownload/src/test/kotlin/com/pouyaheydari/appupdater/directdownload/utils/downloadapk/DownloadManagerRequestCreatorTest.ktdirectdownload/src/test/kotlin/com/pouyaheydari/appupdater/directdownload/utils/permission/DownloadAPKPermissionFactoryTest.ktdirectdownload/src/test/kotlin/com/pouyaheydari/appupdater/directdownload/utils/permission/DownloadAPKPermissionForOAndBelowTest.ktgradle/gradle-daemon-jvm.propertiesgradle/libs.versions.tomlgradle/wrapper/gradle-wrapper.propertiessettings.gradle.ktsstore/consumer-rules.prostore/src/main/java/com/pouyaheydari/appupdater/store/domain/AppStoreCallback.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreFactory.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreIntentBuilder.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreListItem.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreManager.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/AmazonAppStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/AppStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/AppStoreType.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/Aptoide.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/CafeBazaarStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/FDroid.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/GooglePlayStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/HuaweiAppGallery.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/LenovoAppCenter.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/MiGetAppStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/MyketStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/NineApps.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/OneStoreAppMarket.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/OppoAppMarket.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/SamsungGalaxyStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/TencentAppStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/VAppStore.ktstore/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/ZTEAppCenter.kt
💤 Files with no reviewable changes (1)
- build.gradle.kts
Comment on lines
13
to
+55
| */ | ||
| inline fun store(block: StoreListItem.() -> Unit): StoreListItem = StoreListItem().apply(block) | ||
| class StoreListItemBuilder { | ||
| lateinit var store: AppStore | ||
| var title: String = "" | ||
| var icon: Int = 0 | ||
|
|
||
| fun build(): StoreListItem = StoreListItem(store = store, title = title, icon = icon) | ||
| } | ||
|
|
||
| /** | ||
| * Mutable builder for [DirectDownloadListItem] used in DSL context. | ||
| */ | ||
| class DirectDownloadListItemBuilder { | ||
| var title: String = "" | ||
| var url: String = "" | ||
|
|
||
| fun build(): DirectDownloadListItem = DirectDownloadListItem(title = title, url = url) | ||
| } | ||
|
|
||
| /** | ||
| * Mutable builder for [UpdaterDialogData] used in DSL context. | ||
| */ | ||
| class UpdaterDialogDataBuilder { | ||
| var title: String = "" | ||
| var description: String = "" | ||
| var storeList: List<StoreListItem> = listOf() | ||
| var directDownloadList: List<DirectDownloadListItem> = listOf() | ||
| var isForceUpdate: Boolean = false | ||
| var typeface: Typeface? = null | ||
| var errorWhileOpeningStoreCallback: ((String) -> Unit)? = null | ||
| var theme: Theme = Theme.SYSTEM_DEFAULT | ||
|
|
||
| fun build(): UpdaterDialogData = UpdaterDialogData( | ||
| title = title, | ||
| description = description, | ||
| storeList = storeList, | ||
| directDownloadList = directDownloadList, | ||
| isForceUpdate = isForceUpdate, | ||
| typeface = typeface, | ||
| errorWhileOpeningStoreCallback = errorWhileOpeningStoreCallback, | ||
| theme = theme, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting shared builders to a common module.
StoreListItemBuilder and DirectDownloadListItemBuilder are duplicated between the Compose and Fragment DSL modules. Since both build the same model classes (StoreListItem, DirectDownloadListItem), consider extracting them to a shared module (e.g., :store or a new :dsl-core) to reduce duplication and ensure consistent behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@appupdater/src/main/java/com/pouyaheydari/appupdater/main/dsl/DSLUtils.kt`
around lines 13 - 55, Extract the duplicated builder classes
StoreListItemBuilder and DirectDownloadListItemBuilder into a shared module
(e.g., :store or :dsl-core) and replace the duplicate implementations in the
Compose and Fragment DSL modules with imports from that shared module; update
the build system (module dependency) and package/imports so
UpdaterDialogDataBuilder continues to reference the shared
StoreListItemBuilder/DirectDownloadListItemBuilder types and ensure the shared
builders still produce StoreListItem and DirectDownloadListItem with the same
property names (store, title, icon, url) and build() signatures.
Comment on lines
+14
to
+20
| class StoreListItemBuilder { | ||
| lateinit var store: AppStore | ||
| var title: String = "" | ||
| var icon: Int = 0 | ||
|
|
||
| fun build(): StoreListItem = StoreListItem(store = store, title = title, icon = icon) | ||
| } |
There was a problem hiding this comment.
Avoid lateinit in builder — use nullable with validation for clearer errors.
Same issue as the Compose version: if the user forgets to set store, the UninitializedPropertyAccessException provides poor guidance. Use a nullable property with explicit validation.
Proposed fix
class StoreListItemBuilder {
- lateinit var store: AppStore
+ var store: AppStore? = null
var title: String = ""
var icon: Int = 0
- fun build(): StoreListItem = StoreListItem(store = store, title = title, icon = icon)
+ fun build(): StoreListItem = StoreListItem(
+ store = requireNotNull(store) { "StoreListItemBuilder requires 'store' to be set" },
+ title = title,
+ icon = icon
+ )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| class StoreListItemBuilder { | |
| lateinit var store: AppStore | |
| var title: String = "" | |
| var icon: Int = 0 | |
| fun build(): StoreListItem = StoreListItem(store = store, title = title, icon = icon) | |
| } | |
| class StoreListItemBuilder { | |
| var store: AppStore? = null | |
| var title: String = "" | |
| var icon: Int = 0 | |
| fun build(): StoreListItem = StoreListItem( | |
| store = requireNotNull(store) { "StoreListItemBuilder requires 'store' to be set" }, | |
| title = title, | |
| icon = icon | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@appupdater/src/main/java/com/pouyaheydari/appupdater/main/dsl/DSLUtils.kt`
around lines 14 - 20, Replace the lateinit store property in
StoreListItemBuilder with a nullable var (store: AppStore? = null) and add
explicit validation in build(): check that store is not null and throw a clear
IllegalStateException (e.g., "StoreListItemBuilder.store must be set") before
constructing StoreListItem; update the build() method to use the validated
non-null store when calling StoreListItem(store = ..., title = title, icon =
icon) so callers get a helpful error instead of
UninitializedPropertyAccessException.
Comment on lines
+139
to
+144
|
|
||
| // Persistent states are handled by screenState collector above | ||
| DialogScreenStates.HideUpdateInProgress, | ||
| DialogScreenStates.ShowUpdateInProgress, | ||
| DialogScreenStates.Empty, | ||
| -> { /* handled by screenState */ } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if these DialogScreenStates variants are used anywhere else
echo "Searching for usages of HideUpdateInProgress, ShowUpdateInProgress, Empty..."
rg -n "DialogScreenStates\.(HideUpdateInProgress|ShowUpdateInProgress|Empty)" --type kotlinRepository: HeyPouya/AndroidAppUpdater
Length of output: 569
🏁 Script executed:
fd -e kt -type f | xargs grep -l "sealed class DialogScreenStates" | head -5Repository: HeyPouya/AndroidAppUpdater
Length of output: 240
🏁 Script executed:
fd DialogScreenStates .Repository: HeyPouya/AndroidAppUpdater
Length of output: 160
🏁 Script executed:
cat -n ./appupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/model/DialogScreenStates.ktRepository: HeyPouya/AndroidAppUpdater
Length of output: 788
🏁 Script executed:
rg -n "(HideUpdateInProgress|ShowUpdateInProgress|Empty)" --type kotlin appupdater/src/main/java/com/pouyaheydari/appupdater/Repository: HeyPouya/AndroidAppUpdater
Length of output: 1531
Remove unused DialogScreenStates variants from sealed interface definition.
These three variants (Empty, ShowUpdateInProgress, HideUpdateInProgress) are defined in the sealed interface but never emitted to the sideEffect channel anywhere in the codebase. They only appear in the dead code branch at lines 139–144 of AppUpdaterDialog.kt. Remove the variant definitions from DialogScreenStates.kt and the corresponding dead code branch from the when statement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@appupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/AppUpdaterDialog.kt`
around lines 139 - 144, Remove the unused sealed variants
DialogScreenStates.Empty, DialogScreenStates.ShowUpdateInProgress, and
DialogScreenStates.HideUpdateInProgress from the sealed interface definition in
DialogScreenStates (delete their enum/variant declarations) and delete the dead
when-branch in AppUpdaterDialog.kt that matches those three cases (the branch
containing the comment "handled by screenState"). After removal, search for any
remaining references to those symbols (e.g., usages in other files or imports)
and clean them up, then run a build to ensure no unresolved references remain.
Comment on lines
+41
to
+48
| is DialogScreenIntents.OnDirectLinkClicked -> | ||
| _sideEffect.trySend(DialogScreenStates.DownloadApk(intent.item.url)) | ||
|
|
||
| is DialogScreenIntents.OnStoreClicked -> | ||
| _sideEffect.trySend(DialogScreenStates.OpenStore(intent.item.store)) | ||
|
|
||
| is DialogScreenIntents.OnOpeningStoreFailed -> | ||
| _sideEffect.trySend(DialogScreenStates.ExecuteErrorCallback(intent.store.getUserReadableName())) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider handling trySend result for critical side effects.
trySend() can fail silently if the channel buffer is full (returns ChannelResult.isFailure). For non-critical UI events this is typically acceptable, but if any of these side effects are critical (e.g., DownloadApk), consider logging failures or using send() within a coroutine scope.
Example: Log on failure
_sideEffect.trySend(DialogScreenStates.DownloadApk(intent.item.url))
.onFailure { Log.w(TAG, "Failed to send DownloadApk side effect") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@appupdater/src/main/java/com/pouyaheydari/appupdater/main/ui/AppUpdaterViewModel.kt`
around lines 41 - 48, In AppUpdaterViewModel where DialogScreenIntents are
mapped to _sideEffect.trySend (for cases like
DialogScreenIntents.OnDirectLinkClicked, OnStoreClicked, OnOpeningStoreFailed
producing DialogScreenStates.DownloadApk, OpenStore, ExecuteErrorCallback),
handle the trySend result: inspect the ChannelResult (use onFailure or check
isFailure) and either log the failure (include a TAG and context such as which
DialogScreenState failed) or switch to calling send() from a coroutine scope for
critical events (e.g., DownloadApk) so failures are not dropped silently.
| Both Compose and Fragment APIs offer Kotlin DSL builders for a more expressive syntax. | ||
|
|
||
| AppUpdaterDialog.getInstance(data).show(supportFragmentManager, "UPDATE_DIALOG") | ||
| #### Fragment DSL |
There was a problem hiding this comment.
Fix heading level increment.
The headings "Fragment DSL" and "Compose DSL" use #### (h4) but should use ### (h3) since the parent heading "DSL Builders" is ## (h2). Heading levels should only increment by one level at a time (MD001).
Proposed fix
-#### Fragment DSL
+### Fragment DSL-#### Compose DSL
+### Compose DSLAlso applies to: 183-183
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 155-155: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 155, Update the two heading lines "Fragment DSL" and
"Compose DSL" to use one-level deeper headings (change from #### to ###) so they
correctly follow the parent "DSL Builders" (which is ##) and comply with MD001;
locate the headings by their exact text ("Fragment DSL" and "Compose DSL") in
README.md and replace the four-hash markers with three-hash markers.
Comment on lines
+314
to
322
| ``` | ||
| AndroidAppUpdater/ | ||
| ├── core/ # Theme enum and shared constants (pure Kotlin) | ||
| ├── store/ # Store implementations, StoreFactory, icons | ||
| ├── directdownload/ # APK download via DownloadManager + installation | ||
| ├── appupdater/ # Fragment/XML UI (published as "main") | ||
| ├── compose/ # Jetpack Compose UI | ||
| └── app/ # Sample/demo application | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a language identifier to the code block.
The architecture diagram code block lacks a language specifier (MD040). While ASCII diagrams don't have a specific language, using text or an empty identifier can silence the linter warning.
Proposed fix
-```
+```text
AndroidAppUpdater/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| AndroidAppUpdater/ | |
| ├── core/ # Theme enum and shared constants (pure Kotlin) | |
| ├── store/ # Store implementations, StoreFactory, icons | |
| ├── directdownload/ # APK download via DownloadManager + installation | |
| ├── appupdater/ # Fragment/XML UI (published as "main") | |
| ├── compose/ # Jetpack Compose UI | |
| └── app/ # Sample/demo application | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 314 - 322, Update the fenced code block that starts
with the line "AndroidAppUpdater/" so the opening triple backticks include a
language identifier (e.g., change ``` to ```text) to satisfy MD040; locate the
block containing the ASCII architecture diagram (the lines showing
"AndroidAppUpdater/" and its subdirectories) and add the identifier to the
opening ``` only.
Comment on lines
26
to
28
| fun withPackage(storePackageName: String): Builder { | ||
| require(storePackageName.isNotBlank()) { "Store's package name most not be empty" } | ||
| this.storePackageName = storePackageName |
There was a problem hiding this comment.
Fix typo in exception message (most → must).
Line 27 has a wording typo in the thrown error string.
✏️ Proposed fix
- require(storePackageName.isNotBlank()) { "Store's package name most not be empty" }
+ require(storePackageName.isNotBlank()) { "Store's package name must not be empty" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun withPackage(storePackageName: String): Builder { | |
| require(storePackageName.isNotBlank()) { "Store's package name most not be empty" } | |
| this.storePackageName = storePackageName | |
| fun withPackage(storePackageName: String): Builder { | |
| require(storePackageName.isNotBlank()) { "Store's package name must not be empty" } | |
| this.storePackageName = storePackageName |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@store/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreIntentBuilder.kt`
around lines 26 - 28, In Builder.withPackage (the function that sets
storePackageName) the require message has a typo "most" — update the exception
string to read "must not be empty" (i.e., change the require message for
storePackageName in withPackage to "Store's package name must not be empty") so
the validation message is correct; locate the require(...) call in
Builder.withPackage and replace the message accordingly.
Comment on lines
15
to
+18
| data class StoreListItem( | ||
| var store: AppStore = StoreFactory.getStore(AppStoreType.GOOGLE_PLAY, ""), | ||
| var title: String = "", | ||
| var icon: Int = R.drawable.appupdater_ic_cloud, | ||
| ) : Parcelable { | ||
| private constructor(parcel: Parcel) : this( | ||
| ParcelCompat.readParcelable(parcel, AppStore::class.java.classLoader, AppStore::class.java) ?: StoreFactory.getStore(AppStoreType.GOOGLE_PLAY, ""), | ||
| parcel.readString().orEmpty(), | ||
| parcel.readInt(), | ||
| ) | ||
|
|
||
| override fun writeToParcel(parcel: Parcel, flags: Int) { | ||
| parcel.writeParcelable(store, flags) | ||
| parcel.writeString(title) | ||
| parcel.writeInt(icon) | ||
| } | ||
|
|
||
| override fun describeContents(): Int = 0 | ||
|
|
||
| companion object CREATOR : Parcelable.Creator<StoreListItem> { | ||
| override fun createFromParcel(parcel: Parcel): StoreListItem { | ||
| return StoreListItem(parcel) | ||
| } | ||
|
|
||
| override fun newArray(size: Int): Array<StoreListItem?> { | ||
| return arrayOfNulls(size) | ||
| } | ||
| } | ||
| } | ||
| val store: AppStore, | ||
| val title: String = "", | ||
| val icon: Int = 0, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for usages of StoreListItem constructor to assess impact
rg -n "StoreListItem\(" --type kotlin -A 2Repository: HeyPouya/AndroidAppUpdater
Length of output: 16449
Breaking change: icon default changed from drawable resource to 0, affecting preview and test data.
The icon parameter default was changed from a drawable resource to 0, which breaks code paths that relied on the default icon value. Multiple preview and test files instantiate StoreListItem without explicitly providing the icon parameter, expecting the previous drawable default. These will now render without an icon.
The store parameter is now required (no default value), but all codebase usages already provide it explicitly, so this has limited impact.
Update preview data files to explicitly pass icon values (e.g., R.drawable.appupdater_ic_cloud or a similar default drawable), or document why 0 is the correct new default.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@store/src/main/java/com/pouyaheydari/appupdater/store/domain/StoreListItem.kt`
around lines 15 - 18, The default for StoreListItem.icon was changed to 0 which
breaks previews/tests that relied on the previous drawable default; update any
preview/test instantiations of StoreListItem to pass an explicit drawable
resource (e.g., R.drawable.appupdater_ic_cloud or another appropriate drawable)
when constructing StoreListItem, or alternatively revert/change the default in
the StoreListItem declaration; search for usages of StoreListItem in
preview/test files and add the icon argument to each call to restore the
expected icon behavior.
Comment on lines
+4
to
+15
| import kotlinx.parcelize.Parcelize | ||
|
|
||
| internal const val NINE_APPS_STORE_URL = "nineapps://AppDetail?id=" | ||
| internal const val NINE_APPS_PACKAGE = "com.gamefun.apk2u" | ||
|
|
||
| /** | ||
| * Opens application's page in [9-Apps](https://www.9apps.com/) | ||
| */ | ||
| internal data class NineApps(val packageName: String) : AppStore { | ||
| constructor(parcel: Parcel) : this(parcel.readString().orEmpty()) | ||
|
|
||
| @Parcelize | ||
| internal data class NineApps( | ||
| val packageName: String, | ||
| ) : AppStore { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Modules/files using `@Parcelize` =="
rg -n --glob '**/*.kt' '@Parcelize'
echo
echo "== Gradle files declaring parcelize plugin =="
fd -HI 'build.gradle.kts' | while read -r f; do
if rg -n --no-heading -e 'kotlin-parcelize' -e 'org.jetbrains.kotlin.plugin.parcelize' "$f" >/dev/null; then
echo "-- $f"
rg -n --no-heading -e 'kotlin-parcelize' -e 'org.jetbrains.kotlin.plugin.parcelize' "$f"
fi
done
echo
echo "Expected result: each module containing `@Parcelize` classes has a corresponding parcelize plugin declaration."Repository: HeyPouya/AndroidAppUpdater
Length of output: 2158
Add the Kotlin Parcelize plugin declaration to all affected modules.
@Parcelize is used across 19 files in 3 modules (store, directdownload, appupdater), but the parcelize plugin (kotlin-parcelize or org.jetbrains.kotlin.plugin.parcelize) is not declared in any build.gradle.kts file. This will cause compilation failure. Add the plugin to each module's Gradle configuration:
plugins {
kotlin("plugin.parcelize")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@store/src/main/java/com/pouyaheydari/appupdater/store/domain/stores/NineApps.kt`
around lines 4 - 15, Add the Kotlin Parcelize plugin to every module that uses
`@Parcelize` (modules: store, directdownload, appupdater) so files like NineApps
(class NineApps in NineApps.kt) compile; update each module's Gradle settings to
apply the Kotlin parcelize plugin (the kotlin-parcelize /
org.jetbrains.kotlin.plugin.parcelize plugin) in the plugins block of that
module's build.gradle.kts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update Gradle to version 9.4.0, and enhance ProGuard rules
Summary by CodeRabbit
New Features
Bug Fixes
Improvements