Conversation
|
Warning Rate limit exceeded@Nek-12 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpgrades build tooling and dependencies (Gradle wrapper → 9.2.1, version-catalog bumps), migrates Kotlin JVM target config to the new Gradle plugin API, simplifies Maven publishing DSL, adjusts buildSrc SDKs and compiler flags, adds contributor docs and an update script, and changes wrapper invocation to a direct JAR call. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
Summary of ChangesHello @Nek-12, 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 delivers a crucial maintenance update, advancing the project to version 2.1.1. It primarily focuses on updating the Android SDK to meet current AndroidX requirements and performing a broad refresh of build dependencies and tools. The changes also include significant refinements to Gradle configurations, the introduction of detailed repository guidelines, and a new script to automate dependency updates, all aimed at enhancing project stability, compatibility, and developer experience. Highlights
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 introduces a new AGENTS.md file with repository guidelines and updates the copyright year in README.md. The core changes involve a major upgrade of project dependencies, including Kotlin, Compose, Gradle, Kotest, and various build plugins, as reflected in gradle/libs.versions.toml and the Gradle wrapper. Build configurations were refactored to move jvmTarget settings to kotlin.compilerOptions in app/build.gradle.kts and pro.respawn.android-library.gradle.kts, and deprecated kotlinOptions blocks were removed. Compiler arguments in Config.kt were adjusted, removing WASM-specific flags and updating warning suppression syntax, along with changes to JVM compiler arguments. Publishing configurations in build.gradle.kts were simplified, and versionCatalogUpdate settings were streamlined. The gradle.properties file saw the commenting out of org.gradle.configureondemand due to WASMJS incompatibility, removal of android.experimental.additionalArtifactsInModel, and addition of android.builtInKotlin and android.newDsl flags. The gradlew and gradlew.bat scripts were updated to use -jar instead of -classpath for the Gradle wrapper. A new scripts/update_deps.sh script was added to automate dependency updates. A review comment highlighted a potential version mismatch where the Kotest plugin is hardcoded to 6.0.0.M4 while other Kotest libraries use 6.0.7, suggesting alignment or a dedicated version variable for the plugin.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
build.gradle.kts (1)
94-98: DuplicatedetektPluginsdependencies block.The same three detekt plugins are declared twice — once at lines 94-98 and again at lines 153-157. This redundancy should be removed.
🔎 Proposed fix — remove duplicate block
dependencies { - detektPlugins(rootProject.libs.detekt.formatting) - detektPlugins(rootProject.libs.detekt.compose) - detektPlugins(rootProject.libs.detekt.libraries) projects.run { listOf( core, ).forEach { dokka(it) } } }Keep only the first
dependenciesblock (lines 94-98) and merge thedokkareferences into it, or consolidate both into a single block.Also applies to: 153-157
buildSrc/src/main/kotlin/Config.kt (1)
61-66: This is a binary-incompatible change that violates semantic versioning for a published library.The flag
-jvm-default=no-compatibilitybreaks binary compatibility with code compiled under the old compatibility mode (or with older Kotlin versions). Switching to this mode can causeNoSuchMethodErrorand linkage failures for existing library consumers.For a published library on Maven Central, this requires a major version bump (3.0.0), not a minor one (2.1.1). Either:
- Bump the version to 3.0.0 and prominently document the breaking change in release notes, or
- Revert to
-Xjvm-default=allto maintain binary compatibilityAlso verify that removing
-Xcontext-receiversis intentional.
🤖 Fix all issues with AI Agents
In @buildSrc/src/main/kotlin/Config.kt:
- Line 42: The minSdk bump in Config.kt (const val minSdk) from 21→23 is a
breaking change; confirm it’s intentional and required (e.g., due to AndroidX or
SDK36), and if not revert minSdk back to 21. If intentional, keep const val
minSdk = 23 and: update release notes/changelog with an explicit "Breaking
change: minSdk 23" entry and migration guidance for consumers still on API
21–22, add a brief comment near the minSdk constant explaining the dependency
that forced the raise, and communicate the change in your release/PR description
and any consumer-facing docs (README, upgrade guide) and CI/test matrices.
In @gradle.properties:
- Around line 47-49: These AGP opt-out flags (android.builtInKotlin=false and
android.newDsl=false) are AGP 9+ features but Gradle is 9.2.1 here, so verify
the Android Gradle Plugin version (the com.android.tools.build:gradle coordinate
in your build files) and either upgrade AGP to a 9.x release or remove/adjust
these flags; also document the intended target AGP version (e.g., in README or
as a comment next to the android.builtInKotlin/android.newDsl lines) so the
migration intent is clear.
In @gradle/libs.versions.toml:
- Line 15: The kotest library version "kotest" is 6.0.7 but the Kotest Gradle
plugin is hardcoded as 6.0.0.M4; align them by updating the plugin reference to
use the same version (6.0.7) or update the "kotest" entry to the desired plugin
version so both match, and replace the plugin's inline version with the shared
version reference to avoid future drift.
🧹 Nitpick comments (2)
scripts/update_deps.sh (2)
1-11: Add error handling withset -e.The script lacks error handling. If any command fails, subsequent commands still run. Adding
set -eensures the script exits on first failure.🔎 Proposed fix
#!/bin/bash +set -e GRADLE_VER="$1"
9-10: Dual wrapper invocation is intentional — consider adding a comment.Running
./gradlew wrappertwice is a known pattern: the first call downloads the new wrapper JAR, the second uses the updated JAR to regenerate the scripts. A brief comment would help future maintainers understand this isn't a copy-paste error.🔎 Proposed improvement
+# Run wrapper twice: first downloads new JAR, second regenerates scripts with updated wrapper ./gradlew wrapper --gradle-version "$GRADLE_VER" --distribution-type bin ./gradlew wrapper --gradle-version "$GRADLE_VER" --distribution-type bin
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (14)
AGENTS.mdREADME.mdapp/build.gradle.ktsbuild.gradle.ktsbuildSrc/src/main/kotlin/Config.ktbuildSrc/src/main/kotlin/ConfigureAndroid.ktbuildSrc/src/main/kotlin/ConfigureMultiplatform.ktbuildSrc/src/main/kotlin/pro.respawn.android-library.gradle.ktsgradle.propertiesgradle/libs.versions.tomlgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batscripts/update_deps.sh
💤 Files with no reviewable changes (1)
- buildSrc/src/main/kotlin/ConfigureAndroid.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-05T15:02:20.803Z
Learnt from: Nek-12
Repo: respawn-app/ApiResult PR: 14
File: .github/workflows/ci.yml:28-28
Timestamp: 2025-01-05T15:02:20.803Z
Learning: For Android-based Kotlin projects, the jvmTarget (bytecode version) should remain at version 11 to maintain compatibility.
Applied to files:
buildSrc/src/main/kotlin/pro.respawn.android-library.gradle.ktsbuildSrc/src/main/kotlin/ConfigureMultiplatform.ktgradle.propertiesapp/build.gradle.ktsbuildSrc/src/main/kotlin/Config.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (19)
README.md (1)
96-96: LGTM!Standard copyright year update for the new calendar year.
AGENTS.md (3)
1-10: Good documentation of project structure.Clear documentation of the module organization and directory layout for contributors.
18-28: Clear contribution guidelines.Well-documented testing and commit conventions that provide clear expectations for contributors.
11-17: The documented Gradle commands are accurate and compatible with Gradle 9.2.1. All tasks exist and are properly configured::core:jvmTestis provided by the Kotlin Multiplatform configuration,:app:assembleDebugis a standard Android application task, and bothdetektAllanddetektFormatare explicitly registered in the root build.gradle.kts. No changes needed.gradle.properties (1)
9-10: Reasonable to disable for wasmJS compatibility.Disabling Configure on Demand due to wasmJS incompatibility is justified. The TODO provides clear context. Monitor when wasmJS support improves to potentially re-enable this optimization.
buildSrc/src/main/kotlin/pro.respawn.android-library.gradle.kts (1)
1-1: Import required for experimental API usage.The import of
ExperimentalKotlinGradlePluginApiis necessary for the migration to the new compiler options API.buildSrc/src/main/kotlin/ConfigureMultiplatform.kt (2)
52-57: LGTM! Migration tooutputModuleName.set()is correct.This aligns with the newer Kotlin/Wasm DSL where
moduleNameproperty was replaced withoutputModuleNameprovider-based API.
63-70: Review androidTarget usage — current best practice, future migration to Google plugin noted.The
@Suppress("DEPRECATION")is applied correctly.androidTargetis the current recommended API in Kotlin Multiplatform (the replacement for the legacyandroidtarget), not a temporary workaround. The long-term migration path is to Google'scom.android.kotlin.multiplatform.libraryplugin, but that's future planning, not an urgent refactoring. If you want to add a TODO comment for eventual migration once Google's plugin stabilizes, that's a reasonable housekeeping measure but not required.The
jvmTargetis correctly set toJVM_11(line 38 in Config.kt), which aligns with Android compatibility requirements.build.gradle.kts (2)
100-108: RemovedkeepUnusedLibrariesandkeepUnusedPlugins— intentional?Only
keepUnusedVersions = trueremains. If you have libraries/plugins in the catalog that aren't directly referenced (e.g., used viaaliasin subprojects or for documentation), they may be pruned on the nextversionCatalogUpdaterun.
50-51: The call topublishToMavenCentral()without an explicitSonatypeHostargument is correct. The vanniktech maven-publish plugin now defaults to publishing via Sonatype's Central Portal, and the oldSonatypeHostoverloads have been deprecated. Since the legacy OSSRH hosts (S01, DEFAULT) were shut down on June 30, 2025, this change aligns properly with current Sonatype infrastructure. No action required.gradle/libs.versions.toml (3)
38-40: LGTM on new library additions.Adding
dokka-gradleandkotest-assertions-tablewith proper version refs, and includingkotest-assertions-tablein theunittestbundle is a clean enhancement.Also applies to: 55-55
16-16: Kotlin 2.3.0 is officially released (December 16, 2025); verify compatibility with your other dependencies.Kotlin 2.3.0 is a recent release. Confirm it's compatible with your project's dependencies before merging.
13-13: Android Gradle Plugin 8.13.0 is valid.AGP 8.13.0 is an officially released version per the Android Developers release notes. No action needed.
Likely an incorrect or invalid review comment.
app/build.gradle.kts (1)
10-15: LGTM! ModerncompilerOptionsAPI usage.Migrating from deprecated
kotlinOptionsto the experimentalcompilerOptionsAPI is the correct approach. The@OptIn(ExperimentalKotlinGradlePluginApi::class)annotation properly acknowledges the experimental status. Based on learnings,Config.jvmTargetshould be set to JVM 11 for Android compatibility — ensure this is correctly configured inConfig.kt.gradlew.bat (1)
76-76: LGTM! Standard Gradle 9.x wrapper update.The switch from classpath-based to direct
-jarinvocation is part of the Gradle 9.x wrapper modernization. This is a generated file and the change is expected.gradlew (1)
211-214: LGTM! Standard Gradle 9.x wrapper update.The migration from classpath-based to direct
-jar "$APP_HOME/gradle/wrapper/gradle-wrapper.jar"invocation is expected with Gradle 9.x. This simplifies the script by removing CLASSPATH handling.buildSrc/src/main/kotlin/Config.kt (3)
20-22: LGTM: Version bump to 2.1.1.The patch version increment is correct and aligns with the PR objectives.
40-41: Android SDK 36 is available and stable. ThecompileSdk = 36bump is supported as of January 2026, with the SDK Platform reaching platform stability. No breaking changes are reported in the available documentation.
54-60: The new compiler flag syntax is valid. The-Xwarning-level=DIAGNOSTIC_NAME:disabledformat matches the official Kotlin compiler documentation, and the semantic behavior (disabling these specific warnings) is preserved through the:disabledoption. No changes needed.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40a6429620
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const val compileSdk = 36 | ||
| const val targetSdk = compileSdk | ||
| const val minSdk = 21 | ||
| const val minSdk = 23 |
There was a problem hiding this comment.
The library-wide Config.minSdk was increased from 21 to 23, and configureAndroid() applies that value to every Android target. Publishing 2.1.1 with this change silently drops API 21–22 support: consumers building apps for those devices will now get manifest/installation failures even though this is only a patch bump. If the goal wasn’t to break compatibility, keep the prior minSdk (or at least bump the major/minor and document the requirement).
Useful? React with 👍 / 👎.
|
Superseded by #21 (minor bump). |
Pull request was closed
1 participant
Summary
Testing
./gradlew assemble
./gradlew detektAll
./gradlew :core:jvmTest
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.