Centralize SDK versions #723
Conversation
WalkthroughSwitches hard-coded Android SDK version literals to version-catalog lookups across Gradle build scripts and tests, adds compileSdk/targetSdk/minSdk entries to the version catalog, and inserts a single blank line in gradle.properties. No public API or runtime logic changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Build as build.gradle(.kts) / test config
participant Gradle as Gradle evaluator
participant Catalog as Version Catalog (libs.versions)
Build->>Gradle: reference libs.versions.compileSdk/minSdk/targetSdk
Gradle->>Catalog: request version values
Catalog-->>Gradle: return version strings
Gradle->>Gradle: parse/convert to Int (toInteger()/toInt())
Gradle-->>Build: supply resolved SDK integers for configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (6)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
…m/centralize-compilesdk-version
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
gradle.properties (1)
30-31: Add a TODO to remove suppression once AGP supports API 35 to avoid stale configThe suppression is fine, but it's easy to forget later. Add an explicit TODO and reference the catalog entry so future bumps don’t drift.
-# Suppress AGP compatibility warning for compileSdk 35 with AGP 8.5.2 -android.suppressUnsupportedCompileSdk=35 +# Suppress AGP compatibility warning for compileSdk 35 with AGP 8.5.2 +# TODO: Remove this when the AGP version used here officially supports API 35. +# Keep in sync with gradle/libs.versions.toml -> [versions].compileSdk +android.suppressUnsupportedCompileSdk=35gradle/libs.versions.toml (1)
7-7: Optionally centralize minSdk/targetSdk to prevent drift across modulesIf keeping minSdk/targetSdk consistent across modules matters here, consider adding them to the catalog as well and referencing them similarly. This helps avoid drift as you’ve solved for compileSdk.
roborazzi-accessibility-check/build.gradle (1)
12-12: Optional: Consider centralizing minSdk/targetSdk as wellTarget SDK 32 may be intentional, but if you aim for consistency across modules, you could pull min/target into the catalog too.
roborazzi-junit-rule/build.gradle (1)
12-12: Optional: Centralize minSdk/targetSdk to the catalog to avoid driftSame suggestion as other modules—only if consistency across modules is desired.
include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.kt (1)
111-111: Prefer using the catalog here instead of a hard-coded 35 to avoid driftOther integration test projects already reference libs.versions.compileSdk. Doing the same here removes the need for the “keep in sync” comment and prevents future mismatches.
- compileSdk = 35 // Using hardcoded value in integration test - keep in sync with libs.versions.compileSdk + compileSdk = libs.versions.compileSdk.get().toInt() // Use version catalog to avoid driftinclude-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt (1)
229-229: Reduce drift risk: avoid hardcoding compileSdk in generated test build fileThe hardcoded 35 with a “keep in sync” comment works, but it’s easy to miss during future bumps. Consider deriving this value from libs.versions.toml (or falling back to 35) when generating the file to avoid manual sync.
Example approach (optional):
- Parse gradle/libs.versions.toml to read versions.compileSdk and inject that number into the generated build.gradle.kts.
- Or allow an environment/Gradle property override with a fallback.
For instance, inside the generated build.gradle.kts:
android { // Prefer override from property, fall back to the baked-in default compileSdk = providers.gradleProperty("testCompileSdk") .map(String::toInt) .orElse(35) .get() }Then in the test harness, pass -PtestCompileSdk= if needed.
This removes manual sync burden without affecting the tests’ intent.
sample-generate-preview-tests-multiplatform/build.gradle.kts (1)
28-28: Minor consistency note (optional): centralize via shared android convention if desiredYou already have gradle/android.gradle centralizing compileSdk for Groovy-based modules. If you prefer, you could introduce a precompiled convention plugin for Kotlin DSL modules to set compileSdk from the catalog and apply it here as well. This is purely optional and your current approach is fine.
include-build/roborazzi-core/build.gradle (2)
10-10: Avoid duplicate compileSdk configuration (keep a single source of truth).This module sets compileSdk both via the top-level
android.compileSdk(...)and again inside theandroid { compileSdk ... }block (Line 99). This duplication increases the chance of drift. Unless you have a specific need for both, prefer keeping only theandroid { compileSdk ... }assignment.Apply this diff to remove the redundant top-level call:
-android.compileSdk(libs.versions.compileSdk.get().toInteger())
101-106: Consider centralizing targetSdk as well for consistency.
targetSdk 32is still hardcoded. To fully realize the “single source of truth” approach, consider addingtargetSdkto the version catalog (and/or a shared convention plugin) and referencing it here.include-build/roborazzi-gradle-plugin/src/integrationTest/projects/app/build.gradle.kts (1)
16-16: Optional: centralize targetSdk for consistency.
targetSdk = 33is hardcoded. Consider adding atargetSdkentry to the version catalog (or a convention plugin) and referencing it here to keep all SDK levels centralized.roborazzi-ai-openai/build.gradle (2)
10-10: Remove redundant top-level compileSdk configuration.This file also sets compileSdk both at the top-level and inside the
android {}block (Line 52). Prefer a single declaration to avoid drift; theandroid { compileSdk ... }assignment is sufficient.Apply this diff to drop the top-level call:
-android.compileSdk(libs.versions.compileSdk.get().toInteger())
54-58: Optional: centralize targetSdk.
targetSdk 32is still hardcoded. Consider introducing a catalog entry (e.g.,versions.targetSdk) and using it here for consistency with compileSdk centralization.roborazzi-ai-gemini/build.gradle (2)
10-10: Avoid duplicate compileSdk configuration.Similar to other modules, this sets compileSdk at the top-level and again inside
android {}(Line 47). Keep only one declaration—ideally inside theandroidblock.Apply this diff to remove the redundant line:
-android.compileSdk(libs.versions.compileSdk.get().toInteger())
49-53: Optional: centralize targetSdk.
targetSdk 32remains hardcoded. Suggest adding a targetSdk entry to the version catalog or a shared convention to keep SDK levels consistent across modules.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
gradle.properties(1 hunks)gradle/android.gradle(1 hunks)gradle/libs.versions.toml(1 hunks)include-build/roborazzi-core/build.gradle(2 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.kt(1 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt(1 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/projects/app/build.gradle.kts(1 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/projects/sample-generate-preview-tests/build.gradle.kts(1 hunks)roborazzi-accessibility-check/build.gradle(1 hunks)roborazzi-ai-gemini/build.gradle(2 hunks)roborazzi-ai-openai/build.gradle(2 hunks)roborazzi-junit-rule/build.gradle(1 hunks)sample-generate-preview-tests-multiplatform/build.gradle.kts(1 hunks)sample-generate-preview-tests/build.gradle.kts(1 hunks)
🔇 Additional comments (14)
gradle/libs.versions.toml (1)
7-7: LGTM: Single source of truth for compileSdk via version catalogDefining compileSdk in the catalog and consuming as a string with toInt()/toInteger() across DSLs is a solid approach.
roborazzi-accessibility-check/build.gradle (1)
12-12: LGTM: Consuming compileSdk from catalog (Groovy DSL) is correctUsing libs.versions.compileSdk.get().toInteger() is idiomatic for Groovy DSL and keeps this module in sync.
roborazzi-junit-rule/build.gradle (1)
12-12: LGTM: Catalog-based compileSdk on Groovy DSLAlignment with the version catalog looks good and will simplify future bumps.
sample-generate-preview-tests-multiplatform/build.gradle.kts (1)
28-28: Centralized compileSdk via version catalog — looks goodSwitching to libs.versions.compileSdk keeps this sample aligned with the single source of truth. No issues.
sample-generate-preview-tests/build.gradle.kts (1)
23-23: Good move to version-catalog-driven compileSdkThis aligns the sample with centralized management. Looks correct for Kotlin DSL usage.
gradle/android.gradle (2)
2-2: LGTM: compileSdk now sourced from version catalogUsing libs.versions.compileSdk.get().toInteger() in Groovy DSL is correct and consistent with the centralization goal.
2-2: Verify repo-wide consistency and suppression alignment — VERIFIED (no action required)Catalog compileSdk: 35; gradle.properties android.suppressUnsupportedCompileSdk: 35; no hardcoded compileSdk occurrences found in *.gradle / *.gradle.kts.
- gradle/libs.versions.toml → compileSdk = 35
- gradle.properties → android.suppressUnsupportedCompileSdk = 35
- Gradle build scripts (*.gradle, *.gradle.kts) → no hardcoded compileSdk usages detected
include-build/roborazzi-gradle-plugin/src/integrationTest/projects/sample-generate-preview-tests/build.gradle.kts (1)
35-35: Using libs.versions for compileSdk in integration test project — version catalog is configuredVerified: settings.gradle.kts in the integration test project creates a "libs" version catalog from the repo, so libs.versions.compileSdk will resolve.
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/settings.gradle.kts — dependencyResolutionManagement.versionCatalogs.create("libs") { from(files("$roborazziRootPath/gradle/libs.versions.toml")) } (lines ~9–12)
- Usage locations:
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/sample-generate-preview-tests/build.gradle.kts — compileSdk = libs.versions.compileSdk.get().toInt() (line 35)
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/app/build.gradle.kts — compileSdk = libs.versions.compileSdk.get().toInt() (line 11)
include-build/roborazzi-core/build.gradle (2)
99-99: Looks good: using the version catalog for compileSdk is aligned with the PR goals.This correctly centralizes compileSdk resolution via
libs.versions.compileSdk.get().toInteger()in Groovy DSL.
10-10: AGP compileSdk suppression verified — no action required.Verified that the repository already sets the suppression and the version catalog entry:
- gradle.properties — line 31: android.suppressUnsupportedCompileSdk=35
- gradle/libs.versions.toml — line 7: compileSdk = "35"
- include-build/roborazzi-core/build.gradle — line 10: android.compileSdk(libs.versions.compileSdk.get().toInteger())
include-build/roborazzi-gradle-plugin/src/integrationTest/projects/app/build.gradle.kts (2)
11-11: LGTM: compileSdk now references the version catalog.Using
libs.versions.compileSdk.get().toInt()here keeps integration tests aligned with the project-wide setting.
11-11: Integration test projects already wire the root version catalog — no action requiredsettings.gradle.kts in the integration test fixture creates a "libs" catalog pointing to the repo TOML, so the libs.* usages will resolve:
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/settings.gradle.kts
- creates("libs") { from(files("$roborazziRootPath/gradle/libs.versions.toml")) } (roborazziRootPath defaults to "../../../../..")
- Files using libs.* (no local libs.versions.toml found under the test projects):
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/build.gradle.kts (plugins / libs.versions.*)
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/sample-generate-preview-tests/build.gradle.kts (compileSdk = libs.versions.compileSdk...)
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/app/build.gradle.kts (compileSdk = libs.versions.compileSdk..., multiple implementation/testImplementation entries using libs.*)
Result: the integration tests are already configured to resolve the root version catalog; no changes required.
roborazzi-ai-openai/build.gradle (1)
52-52: LGTM: compileSdk is now sourced from the version catalog.This matches the centralization approach and keeps the value consistent.
roborazzi-ai-gemini/build.gradle (1)
47-47: LGTM: compileSdk resolved from the version catalog.Consistent with the PR’s centralization goal.
gradle/libs.versions.toml
Outdated
| @@ -4,6 +4,7 @@ javaToolchain = "17" | |||
| javaTarget = "11" | |||
| # https://www.jetbrains.com/help/kotlin-multiplatform-dev/multiplatform-compatibility-guide.html#version-compatibility | |||
| agp = "8.5.2" | |||
| compileSdk = "35" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify no leftover hard-coded compileSdk values remain
Quick scan to ensure there aren’t stray hard-coded compileSdk ints still lurking in Gradle files (Groovy and KTS).
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show any hard-coded numeric compileSdk usages in build files (exclude build outputs).
# Matches:
# - android.compileSdk(34)
# - compileSdk = 34
# - compileSdk 34
rg -nP -C2 --glob '!**/build/**' '(?x)
(android\.compileSdk\s*\(\s*\d+\s*\)) |
(\bcompileSdk\s*=\s*\d+) |
(\bcompileSdk\s+\d+)
' -g '**/*.gradle' -g '**/*.gradle.kts' | rg -nPv 'libs\.versions\.compileSdk'Length of output: 841
Fix leftover hard-coded compileSdk values
Found hard-coded compileSdk ints; please replace them with the version-catalog value (libs.versions.compileSdk):
- sample-android/build.gradle:18 —
compileSdk 35 - sample-android-without-compose/build.gradle:9 —
compileSdk 35
Suggested replacement:
- Groovy (build.gradle):
compileSdk = libs.versions.compileSdk.get().toInteger() - KTS (build.gradle.kts):
compileSdk = libs.versions.compileSdk.get().toInt()
🤖 Prompt for AI Agents
In gradle/libs.versions.toml around lines 7 to 7 and the referenced build files
(sample-android/build.gradle line 18 and
sample-android-without-compose/build.gradle line 9), replace the hard-coded
compileSdk integer (35) with the version-catalog value; for Groovy build.gradle
files use the version-catalog getter and convert to integer, and for Kotlin
build.gradle.kts files use the version-catalog getter and convert to Int—update
both sample build.gradle files accordingly so they read the value from
libs.versions.compileSdk rather than the literal 35.
…om/roborazzi into tm/centralize-compilesdk-version
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
sample-android-without-compose/build.gradle (1)
9-9: Nit: align DSL style with surrounding propertiesIn this Groovy script, minSdk/targetSdk use the no-assignment style, while compileSdk uses “=”. For consistency, prefer the same style:
- compileSdk = libs.versions.compileSdk.get().toInteger() + compileSdk libs.versions.compileSdk.get().toInteger()sample-android/build.gradle (2)
18-18: Optional: centralize targetSdk/minSdk as well and confirm suppression property
- You can mirror compileSdk’s approach for targetSdk (and optionally minSdk) to simplify future updates.
- Ensure gradle.properties contains android.suppressUnsupportedCompileSdk matching the catalog value, to silence AGP warnings when using API 35.
Use the same repo-wide verification script as posted in the other file’s comment to check for stragglers and the suppression property.
18-18: Nit: keep Groovy DSL style consistentMatch the surrounding android block’s style by dropping the equals:
- compileSdk = libs.versions.compileSdk.get().toInteger() + compileSdk libs.versions.compileSdk.get().toInteger()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
sample-android-without-compose/build.gradle(1 hunks)sample-android/build.gradle(1 hunks)
🔇 Additional comments (2)
sample-android-without-compose/build.gradle (1)
9-9: Centralized compileSdk via version catalog — looks goodUsing libs.versions.compileSdk ensures a single source of truth and simplifies future upgrades.
sample-android/build.gradle (1)
18-18: Centralized compileSdk via version catalog — looks goodConsistent with the repo-wide migration; using libs.versions.compileSdk keeps the sample aligned with other modules.
| @@ -6,7 +6,7 @@ plugins { | |||
|
|
|||
| android { | |||
| namespace 'com.github.takahirom.roborazzi.sample' | |||
| compileSdk 35 | |||
| compileSdk = libs.versions.compileSdk.get().toInteger() | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Optional: also centralize targetSdk/minSdk and verify repo-wide consistency
- Consider adding targetSdk (and possibly minSdk) to the version catalog for the same single-source-of-truth benefits.
- Verify that no modules still hardcode compileSdk and that gradle.properties has the AGP suppression set to the same value as the catalog.
Run the following script to verify:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Confirm version catalog entry exists"
rg -nP '^\s*compileSdk\s*=\s*"[0-9]+"\s*$' gradle/libs.versions.toml || {
echo "Missing compileSdk in libs.versions.toml"; exit 1; }
echo
echo "2) Find any hardcoded compileSdk numbers remaining (should be none)"
rg -nP '^\s*compileSdk\s*(=|\s)\s*\d+\b' -g '!**/build/**' || echo "No hardcoded compileSdk found"
echo
echo "3) Confirm usage of catalog-based compileSdk"
rg -nP 'compileSdk\s*(=|\s)\s*libs\.versions\.compileSdk\.get\(\)\.(toInt|toInteger)\(\)' -g '!**/build/**' || {
echo "No catalog-based compileSdk usage found (unexpected)"; exit 1; }
echo
echo "4) Check AGP suppression is present in gradle.properties (needed for AGP 8.5.x + compileSdk 35)"
rg -nP '^\s*android\.suppressUnsupportedCompileSdk\s*=\s*\d+\s*$' gradle.properties || {
echo "Missing android.suppressUnsupportedCompileSdk in gradle.properties"; exit 1; }Length of output: 2729
Centralize targetSdk/minSdk; compileSdk is cataloged but two integration-test files still hardcode 35
- Verified: gradle/libs.versions.toml contains
compileSdk = "35". - Verified:
gradle.propertiescontainsandroid.suppressUnsupportedCompileSdk=35. - Catalog-based
compileSdkis used across modules (search hits include sample-android, sample-android-without-compose, gradle/android.gradle, roborazzi-* modules, etc.). - Remaining hardcoded occurrences (need attention):
- include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.kt:111
compileSdk = 35 // Using hardcoded value in integration test - keep in sync with libs.versions.compileSdk
- include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt:229
compileSdk = 35 // Using hardcoded value in integration test - keep in sync with libs.versions.compileSdk
- include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.kt:111
- Recommended actions (brief):
- Add
targetSdk(and optionallyminSdk) to the version catalog to centralize them. - Either replace the two hardcoded
35instances in the integration tests with the catalog value (preferred), or keep them but add an automated check / explicit comment referencing the catalog value to avoid drift.
- Add
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
sample-android-without-compose/build.gradle (1)
9-13: Duplicate: verify repo-wide usage and AGP suppression onceThis echoes a prior review: double-check there are no remaining hardcoded compileSdk values and that gradle.properties contains the matching
android.suppressUnsupportedCompileSdkfor API 35.Run:
#!/bin/bash set -euo pipefail echo "compileSdk in catalog:" rg -nP '^\s*compileSdk\s*=\s*"[0-9]+"\s*$' gradle/libs.versions.toml echo echo "Hardcoded compileSdk occurrences (excluding build dirs):" rg -nP '^\s*compileSdk\s*(=|\s)\s*\d+\b' -g '!**/build/**' || echo "None" echo echo "Catalog-based compileSdk usage:" rg -nP 'compileSdk\s*(=|\s)\s*libs\.versions\.compileSdk\.get\(\)\.(toInt|toInteger)\(\)' -g '!**/build/**' echo echo "AGP suppression (for API 35):" rg -nP '^\s*android\.suppressUnsupportedCompileSdk\s*=\s*35\s*$' gradle.properties || echo "Missing (only required if AGP < version that supports 35)"
🧹 Nitpick comments (2)
roborazzi-junit-rule/build.gradle (1)
12-16: Minor style consistency: unify assignment style across modulesThis file uses
compileSdk ...(no=), while other modules in this PR usecompileSdk = .... Consider standardizing to a single style across the repo for readability.include-build/roborazzi-core/build.gradle (1)
10-10: Avoid duplicate compileSdk configurationYou set compileSdk twice: here via
android.compileSdk(...)and again inside theandroid { compileSdk ... }block (Line 99). Keeping one source reduces confusion and maintenance overhead. Recommend removing the top-level call to align with other modules that configure it insideandroid { }.Apply:
-android.compileSdk(libs.versions.compileSdk.get().toInteger())
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
gradle.properties(1 hunks)gradle/android.gradle(1 hunks)gradle/libs.versions.toml(1 hunks)include-build/roborazzi-core/build.gradle(2 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.kt(1 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt(1 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/projects/app/build.gradle.kts(1 hunks)roborazzi-accessibility-check/build.gradle(1 hunks)roborazzi-ai-gemini/build.gradle(2 hunks)roborazzi-ai-openai/build.gradle(2 hunks)roborazzi-junit-rule/build.gradle(1 hunks)sample-android-without-compose/build.gradle(1 hunks)sample-android/build.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- gradle/libs.versions.toml
- roborazzi-ai-openai/build.gradle
- roborazzi-accessibility-check/build.gradle
- gradle.properties
- sample-android/build.gradle
- gradle/android.gradle
- include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt
- roborazzi-ai-gemini/build.gradle
- include-build/roborazzi-gradle-plugin/src/integrationTest/projects/app/build.gradle.kts
- include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.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). (7)
- GitHub Check: ollama-test
- GitHub Check: build
- GitHub Check: compare-screenshot-test
- GitHub Check: store-screenshot-test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: report
🔇 Additional comments (4)
roborazzi-junit-rule/build.gradle (2)
12-12: Good switch to catalog-driven compileSdkUsing libs.versions.compileSdk.get().toInteger() centralizes the value and reduces drift.
15-16: Centralized minSdk/targetSdk look solidResolving from the version catalog improves maintainability and consistency across modules.
sample-android-without-compose/build.gradle (1)
9-13: Nice move to catalog-based SDK versionsAll three SDK values now come from the version catalog, aligning with the PR goals.
include-build/roborazzi-core/build.gradle (1)
99-103: Catalog-based SDK values look good in the android blockConsistent with the rest of the PR; using
libs.versions.*.get().toInteger()is correct for Groovy DSL.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt (1)
234-234: Also source targetSdk (and minSdk) from the version catalog for full centralizationTo fully align with the PR objective and avoid drift, please use the version catalog for targetSdk here (and consider doing the same for minSdk on Line 233).
Apply this diff for targetSdk:
- targetSdk = 35 + targetSdk = libs.versions.targetSdk.get().toInt()Optionally, update minSdk similarly (outside the changed lines):
minSdk = libs.versions.minSdk.get().toInt()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.kt(1 hunks)include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/PreviewGenerateTest.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). (7)
- GitHub Check: build
- GitHub Check: report
- GitHub Check: test
- GitHub Check: ollama-test
- GitHub Check: store-screenshot-test
- GitHub Check: test
- GitHub Check: compare-screenshot-test
🔇 Additional comments (1)
include-build/roborazzi-gradle-plugin/src/integrationTest/java/io/github/takahirom/roborazzi/RoborazziGradleProject.kt (1)
229-229: LGTM: compileSdk now sourced from version catalogUsing libs.versions.compileSdk.get().toInt() removes hardcoded literals and aligns with the PR’s goal of centralizing SDK config.
What
Why
Changes
Summary by CodeRabbit