refactor(01): address plan-checker revisions (1 blocker, 5 warnings)
- 01-02: wave 1→2, depends_on [01]; drop unused androidLibrary classpath entry; guard Kotlin compilerOptions with plugins.withId listeners - 01-05: remove misleading 'gradle exit' echo from verify block - 01-06: harden credential check on docker-compose.yml alone - 01-07: drop hardcoded /Users/rwilk/dev/repo/recipe cd prefix - 01-RESEARCH: rename Open Questions → (RESOLVED); replace 'Recommendation:' with 'RESOLVED:' per gsd Dimension 11 convention
This commit is contained in:
@@ -2,8 +2,8 @@
|
||||
phase: 01-project-infrastructure-module-wiring
|
||||
plan: 02
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
wave: 2
|
||||
depends_on: [01]
|
||||
files_modified:
|
||||
- build-logic/settings.gradle.kts
|
||||
- build-logic/build.gradle.kts
|
||||
@@ -169,7 +169,6 @@ pluginManagement {
|
||||
dependencies {
|
||||
compileOnly(libs.plugins.kotlinMultiplatform.asDependency())
|
||||
compileOnly(libs.plugins.androidApplication.asDependency())
|
||||
compileOnly(libs.plugins.androidLibrary.asDependency())
|
||||
compileOnly(libs.plugins.composeMultiplatform.asDependency())
|
||||
compileOnly(libs.plugins.composeCompiler.asDependency())
|
||||
compileOnly(libs.plugins.composeHotReload.asDependency())
|
||||
@@ -209,11 +208,23 @@ pluginManagement {
|
||||
}
|
||||
}
|
||||
|
||||
// D-11 redundancy guard: if a module applies recipe.quality WITHOUT recipe.kotlin.multiplatform
|
||||
// (e.g. a future pure-JVM utility), ensure allWarningsAsErrors still applies.
|
||||
tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompilationTask<*>>().configureEach {
|
||||
compilerOptions {
|
||||
allWarningsAsErrors.set(true)
|
||||
// D-11 redundancy guard: if a module applies recipe.quality alongside a Kotlin plugin
|
||||
// (multiplatform or jvm), ensure allWarningsAsErrors still applies even if the module
|
||||
// build didn't already configure it. Guarded with plugins.withId so this plugin is
|
||||
// safely composable even when applied alone (no KotlinCompilationTask type available
|
||||
// on the classpath until a Kotlin plugin is present).
|
||||
plugins.withId("org.jetbrains.kotlin.multiplatform") {
|
||||
tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompilationTask<*>>().configureEach {
|
||||
compilerOptions {
|
||||
allWarningsAsErrors.set(true)
|
||||
}
|
||||
}
|
||||
}
|
||||
plugins.withId("org.jetbrains.kotlin.jvm") {
|
||||
tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompilationTask<*>>().configureEach {
|
||||
compilerOptions {
|
||||
allWarningsAsErrors.set(true)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
@@ -425,7 +436,7 @@ pluginManagement {
|
||||
- `build-logic/settings.gradle.kts` ends with `rootProject.name = "build-logic"`
|
||||
- `build-logic/build.gradle.kts` contains `` `kotlin-dsl` `` (triple-backtick plugin alias)
|
||||
- `build-logic/build.gradle.kts` defines the `Provider<PluginDependency>.asDependency()` extension function
|
||||
- `build-logic/build.gradle.kts` has exactly 10 `compileOnly(libs.plugins.*.asDependency())` calls (kotlinMultiplatform, androidApplication, androidLibrary, composeMultiplatform, composeCompiler, composeHotReload, kotlinJvm, ktor, spotless, flywayPlugin)
|
||||
- `build-logic/build.gradle.kts` has exactly 9 `compileOnly(libs.plugins.*.asDependency())` calls (kotlinMultiplatform, androidApplication, composeMultiplatform, composeCompiler, composeHotReload, kotlinJvm, ktor, spotless, flywayPlugin) — no `androidLibrary` because no precompiled plugin applies `com.android.library`; `shared/build.gradle.kts` applies that alias directly
|
||||
- `recipe.kotlin.multiplatform.gradle.kts` contains `id("org.jetbrains.kotlin.multiplatform")` (exactly ONCE, in the plugins block)
|
||||
- `recipe.kotlin.multiplatform.gradle.kts` contains `baseName = "ComposeApp"` (D-20 / PITFALL #10)
|
||||
- `recipe.kotlin.multiplatform.gradle.kts` contains `jvmToolchain(21)` AND `JvmTarget.JVM_11` AND `JvmTarget.JVM_21` (D-08 split)
|
||||
|
||||
@@ -432,7 +432,7 @@ Current server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt (to replace):
|
||||
- DROP wildcard imports `io.ktor.client.request.*`, `io.ktor.client.statement.*`, `io.ktor.http.*`, `io.ktor.server.testing.*`, `kotlin.test.*`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>test -f server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'health endpoint returns 200' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'configureRouting()' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -q 'Database.migrate' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'install(ContentNegotiation)' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'client.get("/health")' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'HttpStatusCode.OK' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -q 'testRoot' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -q 'Greeting().greet()' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -qE 'import kotlin\.test\.\*' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && cd /Users/rwilk/dev/repo/recipe && ./gradlew :server:test --tests "*health*" -q 2>&1 | tail -5 && echo "gradle exit: $?"</automated>
|
||||
<automated>test -f server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'health endpoint returns 200' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'configureRouting()' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -q 'Database.migrate' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'install(ContentNegotiation)' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'client.get("/health")' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && grep -q 'HttpStatusCode.OK' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -q 'testRoot' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -q 'Greeting().greet()' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ! grep -qE 'import kotlin\.test\.\*' server/src/test/kotlin/dev/ulfrx/recipe/ApplicationTest.kt && ./gradlew :server:test --tests "*health*" -q</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `ApplicationTest.kt` defines exactly one `@Test` method whose name contains `health` (case-insensitive)
|
||||
|
||||
@@ -148,7 +148,7 @@ From README.md current content:
|
||||
- `docker-compose.yml` has a `healthcheck:` block using `pg_isready -U recipe -d recipe`
|
||||
- `docker-compose.yml` does NOT contain a `version:` key (modern compose v2)
|
||||
- `docker-compose.yml` does NOT define any service other than `postgres` (D-17: Authentik stays on homelab)
|
||||
- Credentials match Plan 05's `application.conf` defaults (cross-check: `grep 'user = "recipe"' server/src/main/resources/application.conf` and `grep 'password = "recipe"' server/src/main/resources/application.conf` both return 1 line each — if Plan 05 is not complete, skip this sub-check)
|
||||
- `docker-compose.yml` credentials are the exact literals that Plan 05 hardcodes in `application.conf`: `grep -c '^\s*POSTGRES_\(DB\|USER\|PASSWORD\): recipe$' docker-compose.yml` returns `3` (DB, USER, PASSWORD all equal `recipe`). This is enforced on docker-compose.yml alone — the shared hardcoded contract (`recipe/recipe/recipe`) is stated identically in both plans' interfaces, so no cross-file lookup is required.
|
||||
</acceptance_criteria>
|
||||
<done>docker-compose.yml ships postgres:16 matching application.conf defaults; single-service compose file.</done>
|
||||
</task>
|
||||
|
||||
@@ -221,7 +221,7 @@ Phase gate commands (from 01-VALIDATION.md § Sampling Rate):
|
||||
- Do NOT run `./gradlew :server:run` here — it would call `Database.migrate()` which requires a running Postgres. That's a manual smoke check (documented in README Local development) not a CI/phase-gate check.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /Users/rwilk/dev/repo/recipe && ./gradlew spotlessApply -q && bash tools/verify-no-version-literals.sh && bash tools/verify-shared-pure.sh && bash tools/verify-ios-flags.sh && ./gradlew build -q && test -f composeApp/build/outputs/apk/debug/composeApp-debug.apk && test -d composeApp/build/bin/iosSimulatorArm64/debugFramework/ComposeApp.framework && ./gradlew check -q</automated>
|
||||
<automated>./gradlew spotlessApply -q && bash tools/verify-no-version-literals.sh && bash tools/verify-shared-pure.sh && bash tools/verify-ios-flags.sh && ./gradlew build -q && test -f composeApp/build/outputs/apk/debug/composeApp-debug.apk && test -d composeApp/build/bin/iosSimulatorArm64/debugFramework/ComposeApp.framework && ./gradlew check -q</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `./gradlew spotlessApply` exits 0
|
||||
|
||||
@@ -1141,32 +1141,32 @@ kotlin.native.binary.objcDisposeOnMain=false
|
||||
|
||||
**If this table is non-empty:** Planner and `/gsd-discuss-phase` already ran; these assumptions are residual and should be verified during implementation, not re-surfaced. Items A4 and A6 are the ones most likely to surprise — include in Wave 0 smoke checks.
|
||||
|
||||
## Open Questions
|
||||
## Open Questions (RESOLVED)
|
||||
|
||||
1. **Should `shared/` keep `com.android.library` directly applied, or rely on `androidTarget` in the `recipe.kotlin.multiplatform` plugin alone?**
|
||||
- What we know: Current template applies `com.android.library` directly. KMP's `androidTarget { }` declares the Android target but doesn't strictly require the android-library plugin for every module — sometimes it does.
|
||||
- What's unclear: Whether dropping `com.android.library` from `shared/` breaks the composeApp Android consumer.
|
||||
- Recommendation: **Keep `com.android.library` applied in `shared/build.gradle.kts` directly in Phase 1.** Build a `recipe.android.library` convention plugin in a future phase if the direct application becomes a pattern. Don't block Phase 1 on this refactor.
|
||||
- RESOLVED: **Keep `com.android.library` applied in `shared/build.gradle.kts` directly in Phase 1.** Build a `recipe.android.library` convention plugin in a future phase if the direct application becomes a pattern. Don't block Phase 1 on this refactor.
|
||||
|
||||
2. **Does `./gradlew build` invoke `flywayMigrate`? Should it?**
|
||||
- What we know: Flyway plugin exposes `flywayMigrate`, `flywayInfo`, etc. as tasks; it does NOT hook them into `build` by default.
|
||||
- What's unclear: Nothing — this is a choice.
|
||||
- Recommendation: **Do NOT wire Flyway tasks into `build` in Phase 1.** Migration is a server-boot concern; the plugin is for CLI ops (developer runs `./gradlew flywayInfo` manually to inspect state). CI integration lands in Phase 11.
|
||||
- RESOLVED: **Do NOT wire Flyway tasks into `build` in Phase 1.** Migration is a server-boot concern; the plugin is for CLI ops (developer runs `./gradlew flywayInfo` manually to inspect state). CI integration lands in Phase 11.
|
||||
|
||||
3. **Should we add `ktor-server-config-yaml` for a `application.yaml` alternative to HOCON?**
|
||||
- What we know: Ktor 3.x supports YAML config via the `ktor-server-config-yaml` artifact; HOCON remains the default.
|
||||
- What's unclear: Team preference.
|
||||
- Recommendation: **Stick with HOCON.** Our server dev is Kotlin/Ktor background (user profile) and HOCON is the historically canonical Ktor config. YAML is a nice-to-have, not worth the added dep.
|
||||
- RESOLVED: **Stick with HOCON.** Our server dev is Kotlin/Ktor background (user profile) and HOCON is the historically canonical Ktor config. YAML is a nice-to-have, not worth the added dep.
|
||||
|
||||
4. **How to verify iOS binary flags take effect without shipping a build to hardware?**
|
||||
- What we know: Simulator launch eliminates most of the visible symptoms of PITFALL #1; Instruments on a real device would be the gold standard.
|
||||
- What's unclear: Whether simulator-level verification is sufficient for Phase 1 sign-off.
|
||||
- Recommendation: **Verify at two levels:** (a) grep `gradle.properties` for the two flags (trivial but catches omission); (b) build the iOS framework and capture the Kotlin/Native link log for a line showing the GC + objcDisposeOnMain options. Real-device verification under Instruments is deferred to Phase 10 (UI chrome) when there's meaningful UI work to stress-test.
|
||||
- RESOLVED: **Verify at two levels:** (a) grep `gradle.properties` for the two flags (trivial but catches omission); (b) build the iOS framework and capture the Kotlin/Native link log for a line showing the GC + objcDisposeOnMain options. Real-device verification under Instruments is deferred to Phase 10 (UI chrome) when there's meaningful UI work to stress-test.
|
||||
|
||||
5. **Does `recipe.quality` need a `targetExclude` for generated Compose Resources code?**
|
||||
- What we know: Compose Multiplatform generates `Res.kt` under `build/generated/compose/resourceGenerator/...`.
|
||||
- What's unclear: Whether Spotless/ktlint visit `build/` by default (they shouldn't, but worth confirming).
|
||||
- Recommendation: **Add `targetExclude("**/build/**", "**/generated/**")` explicitly in the Spotless config** (already in Pattern 5 example) to future-proof against any `.kt` file landing in those paths.
|
||||
- RESOLVED: **Add `targetExclude("**/build/**", "**/generated/**")` explicitly in the Spotless config** (already in Pattern 5 example) to future-proof against any `.kt` file landing in those paths.
|
||||
|
||||
## Environment Availability
|
||||
|
||||
|
||||
Reference in New Issue
Block a user