Sync multimodule with base and fix customizer.sh#202
Sync multimodule with base and fix customizer.sh#202JoseAlcerreca wants to merge 16 commits intomultimodulefrom
Conversation
Change-Id: I507de0d9ff44c8cefa4635068e762079bd238145
* Add renovate.json * Sets branches for Renovate Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Jose Alcérreca <JoseAlcerreca@users.noreply.github.com>
Feedback from Lou: /bin/bash was v3 but `bash` used a v5, installed with homebrew. This also removes the need to chmod.
* Add packaging info to architecture templates * Update screenshot * Rename packaging extension to .yaml instead of .yml
Fixes #95
- Ported edge-to-edge UI improvements and Room schema configuration from base branch. - Updated dependencies to align with base (AGP 9.0.1, Kotlin 2.3.10, etc.). - Migrated all modules from kapt to KSP for Hilt and Room. - Aligned build files with AGP 9.0 conventions (removed redundant Kotlin plugins, updated packaging block). - Fixed customizer.sh: corrected typos, improved sed portability, and made it less destructive by retaining .git and .github directories. - Added explicit versions for Compose libraries in libs.versions.toml to ensure build stability in multi-module setup. Co-authored-by: JoseAlcerreca <609125+JoseAlcerreca@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request brings the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does an excellent job of synchronizing the multimodule branch with the base branch, incorporating modern Android development practices such as KSP, version catalogs, and edge-to-edge UI. The improvements to the customizer.sh script also significantly enhance its usability and robustness. I have identified a few areas for improvement, including an unused function parameter, an incorrect unit test, and some minor inconsistencies in documentation and file naming that could impact maintainability. Overall, this is a strong update that establishes a solid foundation for the template. Addressing the suggested points will further enhance the quality and clarity of the codebase.
| fun MyModelScreen( | ||
| onItemClick: (NavKey) -> Unit, | ||
| modifier: Modifier = Modifier, | ||
| viewModel: MyModelViewModel = hiltViewModel(), | ||
| ) { |
There was a problem hiding this comment.
The onItemClick parameter is declared but remains unused within this composable, nor is it passed to the internal MyModelScreen. This could indicate either missing click handling logic or dead code. If item clicks are not intended, removing this parameter would clarify the component's API. This change would also require an update at the call site in feature-mymodel/src/main/java/android/template/feature/mymodel/navigation/EntryProvider.kt.
fun MyModelScreen(
modifier: Modifier = Modifier,
viewModel: MyModelViewModel = hiltViewModel(),
) {|
|
||
| # License | ||
|
|
||
| Now in Android is distributed under the terms of the Apache License (Version 2.0). See the |
There was a problem hiding this comment.
The phrase "Now in Android is distributed..." appears to be a remnant from another project. It would be clearer to update this to be specific to the current project, for example, "This code is distributed...".
| Now in Android is distributed under the terms of the Apache License (Version 2.0). See the | |
| This code is distributed under the terms of the Apache License (Version 2.0). See the |
| // Enable room auto-migrations | ||
| ksp { | ||
| arg("room.schemaLocation", "$projectDir/schemas") | ||
| } |
There was a problem hiding this comment.
The room.schemaLocation is already configured in the core-database module, which is the appropriate location since it's where the Room database is defined. This declaration in the app module is redundant and could lead to confusion. Removing it would improve the clarity of the build configuration.
| } | ||
|
|
||
| @Test | ||
| fun uiState_onItemSaved_isDisplayed() = runTest { |
There was a problem hiding this comment.
The test uiState_onItemSaved_isDisplayed currently has the same implementation as uiState_initiallyLoading and does not verify that an item is saved and displayed. To accurately reflect its name, the test should be updated to invoke viewModel.addMyModel() and then assert that the uiState correctly transitions to a Success state containing the newly added item. Please note that the FakeMyModelRepository might need to be updated to use a hot flow (like StateFlow) for this test to function as expected.
| components = [SingletonComponent::class], | ||
| replaces = [DataModule::class] | ||
| ) | ||
| interface FakeDataModule { |
There was a problem hiding this comment.
The filename TestDatabaseModule.kt is a bit misleading, as the FakeDataModule within it provides a fake implementation for the data layer (MyModelRepository), not the database layer. For better clarity and consistency, I suggest renaming the file to FakeDataModule.kt to align with the name of the interface it contains.
3 participants
This PR synchronizes the
multimodulebranch with the latest improvements from thebasebranch and fixes several issues in thecustomizer.shscript.Key changes:
MainActivityand appliedsafeDrawingPaddingin navigation entries, consistent with the latest UI improvements in the base branch.room.schemaLocationKSP argument to the correct block incore-database.kotlin-androidandkotlin-kaptplugins (supported natively by AGP 9.0), and migrated annotation processing to KSP across all modules.libs.versions.tomlto match the base branch. Added explicit versions for Compose material3 and ui libraries to fix resolution issues encountered during the build.rm -rf mv).sedcompatibility for both macOS (BSD) and Linux (GNU)..gitand.githubfolders, allowing users to maintain history and CI/CD setup.MyApplicationrenaming logic to cover more files.The changes have been verified by successfully running
./gradlew :app:assembleDebugand./gradlew test.PR created automatically by Jules for task 12297434757777862811 started by @JoseAlcerreca