Skip to content

Setup structure#2131

Closed
mason-doulabi wants to merge 2 commits into
android:mainfrom
mason-doulabi:setup-structure
Closed

Setup structure#2131
mason-doulabi wants to merge 2 commits into
android:mainfrom
mason-doulabi:setup-structure

Conversation

@mason-doulabi

Copy link
Copy Markdown

DO NOT CREATE A PULL REQUEST WITHOUT READING THESE INSTRUCTIONS

Instructions

Thanks for submitting a pull request. To accept your pull request we need you do a few things:

If this is your first pull request

Ensure tests pass and code is formatted correctly

  • Run local tests on the DemoDebug variant by running ./gradlew testDemoDebug
  • Fix code formatting: ./gradlew spotlessApply

Add a description

We need to know what you've done and why you've done it. Include a summary of what your pull request contains, and why you have made these changes. Include links to any relevant issues which it fixes.

Here's an example.

NOW DELETE THIS LINE AND EVERYTHING ABOVE IT

What I have done and why

<add your PR description here>

@mason-doulabi mason-doulabi requested a review from dturner as a code owner June 18, 2026 09:20
@google-cla

google-cla Bot commented Jun 18, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new set of modules for an IMDb-specific application, including the main application module (app-imbd) and various core and feature modules (i_core and i_feature). Feedback on these changes highlights several critical build and configuration issues: multiple build files contain invalid compileSdk configurations and should leverage the custom convention plugin, and direct loading of local.properties in the network module will fail on CI/CD without safety checks. Additionally, the app-imbd module is missing dependencies on the new modules, there is a package name mismatch between imdb and imbd, and several serialization plugin declarations should be migrated to the version catalog. Finally, minor improvements are suggested to update the parent theme to Material 3, make the app name user-friendly, and align the domain module's namespace for consistency.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +1 to +22
plugins {
alias(libs.plugins.android.library)
}

android {
namespace = "com.smmousavi.i_core.common"
compileSdk {
version = release(36)
}

defaultConfig {
minSdk = 24

testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
consumerProguardFiles("consumer-rules.pro")
}
compileOptions {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The compileSdk block uses version = release(36), which is invalid Gradle Kotlin DSL and will cause compilation errors. Additionally, instead of manually configuring standard Android library properties, you should use the custom nowinandroid.android.library convention plugin to ensure consistent configuration (compileSdk, minSdk, Java version, etc.) across all modules in the project.

plugins {
    alias(libs.plugins.nowinandroid.android.library)
}

android {
    namespace = "com.smmousavi.i_core.common"
}

Comment on lines +1 to +22
plugins {
alias(libs.plugins.android.library)
}

android {
namespace = "com.smmousavi.i_core.database"
compileSdk {
version = release(36)
}

defaultConfig {
minSdk = 24

testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
consumerProguardFiles("consumer-rules.pro")
}
compileOptions {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The compileSdk block uses version = release(36), which is invalid Gradle Kotlin DSL and will cause compilation errors. Additionally, instead of manually configuring standard Android library properties, you should use the custom nowinandroid.android.library convention plugin to ensure consistent configuration (compileSdk, minSdk, Java version, etc.) across all modules in the project.

plugins {
    alias(libs.plugins.nowinandroid.android.library)
}

android {
    namespace = "com.smmousavi.i_core.database"
}

Comment on lines +1 to +22
plugins {
alias(libs.plugins.android.library)
}

android {
namespace = "com.smmousavi.i_core.designsystem"
compileSdk {
version = release(36)
}

defaultConfig {
minSdk = 24

testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
consumerProguardFiles("consumer-rules.pro")
}
compileOptions {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The compileSdk block uses version = release(36), which is invalid Gradle Kotlin DSL and will cause compilation errors. Additionally, instead of manually configuring standard Android library properties, you should use the custom nowinandroid.android.library convention plugin to ensure consistent configuration (compileSdk, minSdk, Java version, etc.) across all modules in the project.

plugins {
    alias(libs.plugins.nowinandroid.android.library)
}

android {
    namespace = "com.smmousavi.i_core.designsystem"
}

Comment on lines +1 to +22
plugins {
alias(libs.plugins.android.library)
}

android {
namespace = "com.smmousavi.i_core.presentation"
compileSdk {
version = release(36)
}

defaultConfig {
minSdk = 24

testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
consumerProguardFiles("consumer-rules.pro")
}
compileOptions {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The compileSdk block uses version = release(36), which is invalid Gradle Kotlin DSL and will cause compilation errors. Additionally, instead of manually configuring standard Android library properties, you should use the custom nowinandroid.android.library convention plugin to ensure consistent configuration (compileSdk, minSdk, Java version, etc.) across all modules in the project.

plugins {
    alias(libs.plugins.nowinandroid.android.library)
}

android {
    namespace = "com.smmousavi.i_core.presentation"
}

Comment on lines +1 to +22
plugins {
alias(libs.plugins.android.library)
}

android {
namespace = "com.smmousavi.i_feature.search"
compileSdk {
version = release(36)
}

defaultConfig {
minSdk = 24

testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
consumerProguardFiles("consumer-rules.pro")
}
compileOptions {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The compileSdk block uses version = release(36), which is invalid Gradle Kotlin DSL and will cause compilation errors. Additionally, instead of manually configuring standard Android library properties, you should use the custom nowinandroid.android.library convention plugin to ensure consistent configuration (compileSdk, minSdk, Java version, etc.) across all modules in the project.

plugins {
    alias(libs.plugins.nowinandroid.android.library)
}

android {
    namespace = "com.smmousavi.i_feature.search"
}

@@ -0,0 +1,10 @@
plugins {
alias(libs.plugins.nowinandroid.jvm.library)
id("kotlinx-serialization")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Instead of using the hardcoded plugin ID kotlinx-serialization, use the version catalog alias alias(libs.plugins.kotlin.serialization) for consistency and centralized version management.

    alias(libs.plugins.kotlin.serialization)

alias(libs.plugins.nowinandroid.android.library)
alias(libs.plugins.nowinandroid.android.library.jacoco)
alias(libs.plugins.nowinandroid.hilt)
id("kotlinx-serialization")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Instead of using the hardcoded plugin ID kotlinx-serialization, use the version catalog alias alias(libs.plugins.kotlin.serialization) for consistency and centralized version management.

    alias(libs.plugins.kotlin.serialization)

<?xml version="1.0" encoding="utf-8"?>
<resources>

<style name="Theme.Nowinandroid" parent="android:Theme.Material.Light.NoActionBar" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The parent theme is set to android:Theme.Material.Light.NoActionBar, which is an ancient platform theme. For a modern Jetpack Compose app using Material 3, it should inherit from Theme.Material3.DayNight.NoActionBar to ensure proper styling, dark mode support, and compatibility with edge-to-edge system UI.

Suggested change
<style name="Theme.Nowinandroid" parent="android:Theme.Material.Light.NoActionBar" />
<style name="Theme.Nowinandroid" parent="Theme.Material3.DayNight.NoActionBar" />

@@ -0,0 +1,3 @@
<resources>
<string name="app_name">app-imbd</string>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The app_name is set to "app-imbd". This is a user-facing string and should be a user-friendly name like "IMDb".

Suggested change
<string name="app_name">app-imbd</string>
<string name="app_name">IMDb</string>

}

android {
namespace = "com.smmousavi.domain"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The namespace for the :i_core:domain module is com.smmousavi.domain, whereas all other core modules use the com.smmousavi.i_core.* prefix (e.g., com.smmousavi.i_core.common, com.smmousavi.i_core.database). It is recommended to rename the package and namespace to com.smmousavi.i_core.domain for consistency.

    namespace = "com.smmousavi.i_core.domain"

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.

1 participant