feat(kotlin-sdk): add maven-publish for the release AAR#4045
Conversation
Publishes org.dashfoundation:dash-sdk-android with sources jar and POM (Room/DataStore/Biometric api deps carried). publishToMavenLocal works today; a remote repository block can be added when a hosting decision is made. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔍 Review in progress — actively reviewing now (commit ffeeb82) |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe Kotlin SDK's build.gradle.kts is updated to enable Maven publishing. It applies the maven-publish plugin, sets group/version from the sdkVersion property, configures Android release-variant publishing with a sources JAR, and defines a Maven publication with POM metadata (name, description, URL, MIT license). ChangesMaven Publishing Configuration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/kotlin-sdk/sdk/build.gradle.kts (1)
96-125: 📐 Maintainability & Code Quality | 🔵 TrivialAdd POM
scmanddeveloperssections before publishing to a remote repository.The POM includes name, description, URL, and license — sufficient for
publishToMavenLocal. Maven Central and most remote repositories also requirescmanddeveloperssections for validation. Since remote publication is intentionally deferred, these can be added when a hosting decision is made.📋 POM additions for future remote publication
pom { name.set("Dash Platform Kotlin SDK") description.set( "Kotlin SDK for Dash Core (L1 SPV) and Dash Platform " + "(identities, DPNS, DashPay, shielded balances)" ) url.set("https://github.com/dashpay/platform") licenses { license { name.set("MIT License") url.set("https://github.com/dashpay/platform/blob/master/LICENSE") } } + scm { + url.set("https://github.com/dashpay/platform") + connection.set("scm:git:git://github.com/dashpay/platform.git") + developerConnection.set("scm:git:ssh://git@github.com/dashpay/platform.git") + } + developers { + developer { + id.set("dashfoundation") + name.set("Dash Foundation") + url.set("https://dashfoundation.org") + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kotlin-sdk/sdk/build.gradle.kts` around lines 96 - 125, The release publication in the `MavenPublication` block under `afterEvaluate { publishing { publications { create<MavenPublication>("release") { pom { ... } } } } }` is missing remote-repository metadata; add `scm` and `developers` entries alongside the existing `name`, `description`, `url`, and `licenses` fields so the POM is ready for remote publication validation when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/kotlin-sdk/sdk/build.gradle.kts`:
- Around line 96-125: The release publication in the `MavenPublication` block
under `afterEvaluate { publishing { publications {
create<MavenPublication>("release") { pom { ... } } } } }` is missing
remote-repository metadata; add `scm` and `developers` entries alongside the
existing `name`, `description`, `url`, and `licenses` fields so the POM is ready
for remote publication validation when needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92a8426b-9585-4d78-a00d-d2bc13bdb215
📒 Files selected for processing (1)
packages/kotlin-sdk/sdk/build.gradle.kts
There was a problem hiding this comment.
Code Review
The PR adds Maven publication for the Kotlin SDK release AAR, but the resulting coordinate is not reliably consumable from a clean checkout. The publication can succeed without packaging the required JNI library, and the published compile API omits coroutines even though public SDK types expose Flow/StateFlow.
Source: reviewers: opus/claude general failed (quota), gpt-5.5/codex general completed; verifier: gpt-5.5/codex; specialists: none.
🔴 2 blocking
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/kotlin-sdk/sdk/build.gradle.kts`:
- [BLOCKING] packages/kotlin-sdk/sdk/build.gradle.kts:104-105: Publishing can create an AAR without the JNI library
The release publication is created directly from the Android release component, but there is no Gradle task dependency or validation that builds `sdk/src/main/jniLibs/<abi>/libdash_sdk_jni.so` first. That directory is gitignored and absent in a clean checkout; the repo docs and build script show it is produced only by `packages/kotlin-sdk/build_android.sh`. Because `NativeLoader.ensureLoaded()` calls `System.loadLibrary("dash_sdk_jni")`, publishing from a clean tree can produce a Maven coordinate that installs successfully but fails at runtime with `UnsatisfiedLinkError`. The publish path should either depend on the native build or fail fast when the expected ABI libraries are missing.
- [BLOCKING] packages/kotlin-sdk/sdk/build.gradle.kts:72: Coroutines core is missing from the published compile API
`kotlinx-coroutines-core` is declared as an `implementation` dependency, so the published component does not expose it on consumers' compile classpaths. The SDK's public API returns coroutines types such as `Flow`, `StateFlow`, and `SharedFlow` from classes including `DataManager`, `ShieldedService`, `WalletManagerStore`, and `PlatformWalletManager`. A consumer depending only on `org.dashfoundation:dash-sdk-android` can therefore fail to compile unless it independently declares coroutines. Since this PR makes the SDK consumable through a Maven coordinate, public API dependencies need to be published as `api` dependencies.
| create<MavenPublication>("release") { | ||
| from(components["release"]) |
There was a problem hiding this comment.
🔴 Blocking: Publishing can create an AAR without the JNI library
The release publication is created directly from the Android release component, but there is no Gradle task dependency or validation that builds sdk/src/main/jniLibs/<abi>/libdash_sdk_jni.so first. That directory is gitignored and absent in a clean checkout; the repo docs and build script show it is produced only by packages/kotlin-sdk/build_android.sh. Because NativeLoader.ensureLoaded() calls System.loadLibrary("dash_sdk_jni"), publishing from a clean tree can produce a Maven coordinate that installs successfully but fails at runtime with UnsatisfiedLinkError. The publish path should either depend on the native build or fail fast when the expected ABI libraries are missing.
source: ['codex']
There was a problem hiding this comment.
indeed, we will need to integrate building of the JNI libs into the gradle build system. It can be done by running the build_android.sh script from the grade script or using CMake. There could be other better ways.
There was a problem hiding this comment.
Resolved in 3d05a4f — Publishing can create an AAR without the JNI library no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
HashEngineering
left a comment
HashEngineering
left a comment
There was a problem hiding this comment.
comments left regarding future changes to support publishing to Maven Central.
| `maven-publish` | ||
| } | ||
|
|
||
| group = "org.dashfoundation" |
There was a problem hiding this comment.
one issue with this name is that http://dashfoundation.org is owned by someone else. To get our artifacts published on Maven Central, we need to own the domain that goes with this group id.
We currently own dashj.org and are publishing there all of our legacy platform and core related SDK libraries. This may not be ideal for this new SDK.
There was a problem hiding this comment.
This is good for now, but still requires a local build process to put the Kotlin SDK libraries into the local Maven Repository. It does not allow for publishing to Maven Central, where everyone who wants to build an app that consumes the Kotlin SDK can access it without cloning this repo and building the Kotlin SDK.
Recently, on Android, we have had two outside contributors.
…api; POM scm/developers Address review of the release-AAR maven-publish. - Fail the publish tasks (verifyJniLibsPresent, wired to publishToMavenLocal / publishToMavenRepository) when src/main/jniLibs/<abi>/libdash_sdk_jni.so is absent for the arm64-v8a / x86_64 ABI policy build_android.sh produces. That directory is gitignored and empty in a clean checkout, so without this a publish could ship a coordinate that installs but dies at runtime with UnsatisfiedLinkError. Escape hatch: -PallowMissingJni warns and continues (metadata-only dry run). - Move kotlinx-coroutines-core from implementation to api: the public SDK surface returns Flow/StateFlow (Room DAOs, PlatformWalletManager.pendingIdentityKeys), so consumers of the coordinate need it on their compile classpath. - Add POM scm + developers sections (required for Maven Central / remote POM validation), pointed at github.com/dashpay/platform rather than a vanity domain, plus a note that the org.dashfoundation groupId can't go to Maven Central until the matching domain is owned (per HashEngineering). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-Forward Prior Findings
Both prior blocking findings are FIXED at head 3d05a4f: coroutines-core is now published as an api dependency (build.gradle.kts:84), and a verifyJniLibsPresent task now gates both PublishToMavenLocal and PublishToMavenRepository tasks (build.gradle.kts:166-185), failing a clean-tree publish unless -PallowMissingJni is passed. Neither is carried forward as an open finding.
New Findings In Latest Delta
The latest delta (b27971b..3d05a4f) only touched the coroutines-core line and added SCM/developer POM metadata plus the JNI guard task — no new issues found there. Codex additionally raised a cumulative (pre-existing, untouched-by-delta) concern that kotlinx-serialization-json remains implementation rather than api. Verified against source: the only public @Serializable type is Sdk.MasternodeEntry, but it is never returned by any public function (only consumed by internal fun parseActiveMasternodes and a private envelope class) — the actually-public ActiveMasternode return type isn't @Serializable. This is a real but narrow edge case (a consumer would only hit it by explicitly calling Sdk.MasternodeEntry.serializer()), unlike the coroutines-core issue which affected the SDK's primary, pervasively-used reactive surface (Room Flow, StateFlow properties across many public classes). Downgraded from blocking to suggestion accordingly; an equally valid fix is marking MasternodeEntry internal since it looks like a wire-format DTO that leaked to public visibility rather than an intentional serialization API.
Source (experiment sonnet-primary-opus-quarter-sample-20260710, cohort sonnet_primary, bucket 1): reviewers codex/general=gpt-5.6-sol(completed); sonnet5/general=claude-sonnet-5(completed); verifier=verifier-sonnet5-4045-1783737274=claude-sonnet-5; orchestrator=openai/gpt-5.6-sol reasoning=high (orchestration-only, not a reviewer/verifier).
🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/kotlin-sdk/sdk/build.gradle.kts`:
- [SUGGESTION] packages/kotlin-sdk/sdk/build.gradle.kts:86: kotlinx-serialization-json is runtime-only despite a public @Serializable type
`kotlinx-serialization-json` is declared with `implementation`, so it won't be on the compile classpath of consumers pulling the new Maven coordinate. `Sdk.MasternodeEntry` (Sdk.kt:127-133) is a public, default-visibility data class annotated `@Serializable`, so the serialization compiler plugin generates a public `serializer(): KSerializer<MasternodeEntry>` on it — a consumer calling that would fail to compile without kotlinx-serialization-core on their classpath. That said, this is narrower than the coroutines-core issue: `MasternodeEntry` is never returned by any public function — only `internal fun parseActiveMasternodes` and a `private` envelope class touch it, and the actually-public `discoverActiveMasternodes()` return type (`ActiveMasternode`) isn't `@Serializable`. A typical consumer is very unlikely to hit this. The cleaner fix may be marking `MasternodeEntry` `internal` (it reads like a wire-format DTO that leaked to public visibility) rather than promoting the dependency to `api`.
| // coroutines-core on its own compile classpath. | ||
| api(libs.kotlinx.coroutines.core) | ||
| implementation(libs.kotlinx.coroutines.android) | ||
| implementation(libs.kotlinx.serialization.json) |
There was a problem hiding this comment.
🟡 Suggestion: kotlinx-serialization-json is runtime-only despite a public @serializable type
kotlinx-serialization-json is declared with implementation, so it won't be on the compile classpath of consumers pulling the new Maven coordinate. Sdk.MasternodeEntry (Sdk.kt:127-133) is a public, default-visibility data class annotated @Serializable, so the serialization compiler plugin generates a public serializer(): KSerializer<MasternodeEntry> on it — a consumer calling that would fail to compile without kotlinx-serialization-core on their classpath. That said, this is narrower than the coroutines-core issue: MasternodeEntry is never returned by any public function — only internal fun parseActiveMasternodes and a private envelope class touch it, and the actually-public discoverActiveMasternodes() return type (ActiveMasternode) isn't @Serializable. A typical consumer is very unlikely to hit this. The cleaner fix may be marking MasternodeEntry internal (it reads like a wire-format DTO that leaked to public visibility) rather than promoting the dependency to api.
| implementation(libs.kotlinx.serialization.json) | |
| api(libs.kotlinx.serialization.json) |
source: ['codex']
There was a problem hiding this comment.
Resolved in fcaa91f — kotlinx-serialization-json is runtime-only despite a public @serializable type no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
|
Fixed. Publishing now fails fast via |
|
@HashEngineering — proposal for the publishing coordinates, to resolve the Maven Central namespace question: use |
|
in our group discussion we decided to continue using the We will still need to use the 'maven-publish' plugin to support publishing other the local repo on the dev's system. |
…l/Sonatype deploy Per HashEngineering's decision on PR dashpay#4045, switch the published Maven coordinate group from org.dashfoundation to org.dashj. Dash already owns dashj.org and ships its legacy Core/Platform SDKs there, so the org.dashj group needs no separate domain verification for Maven Central; the org.dashfoundation group would have (dashfoundation.org is third-party-held). This is a coordinates-only change: the Kotlin source packages stay org.dashfoundation.dashsdk, and the artifactId stays dash-sdk-android. Add a jreleaser deploy config mirroring dashj/core/build.gradle: maven-publish stages the signed artifacts into build/staging-deploy, and jreleaserDeploy uploads release versions to Maven Central (Sonatype portal) and -SNAPSHOT versions to the Sonatype snapshots repo. Signing + Sonatype credentials are read from jreleaser env/props at deploy time only. maven-publish still drives publishToMavenLocal / internal-repo publishing. The gradle signing plugin is guarded to require a key only when a real PublishToMavenRepository task is in the graph, so publishToMavenLocal and plain assembles need no GPG setup. Validated: `./gradlew :sdk:publishToMavenLocal` lands org.dashj:dash-sdk-android:0.1.0-SNAPSHOT under ~/.m2/repository/org/dashj/ with signReleasePublication SKIPPED. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Prior Finding Reconciliation
- STILL VALID:
kotlinx-serialization-jsonremainsimplementationwhileSdk.MasternodeEntryis still a public@Serializabletype.
Carried-Forward Prior Findings
- The serialization compile-classpath suggestion remains open at the current head and is carried forward below.
New Findings In Latest Delta
- Two blockers in the new remote-publishing path: the release publication stages no required Javadoc JAR, and the documented in-memory Gradle signing properties are not wired through
useInMemoryPgpKeys.
Additional Cumulative Findings
None.
Source: Sol reviewer gpt-5.6-sol (general, completed); Sonnet reviewer claude-sonnet-5 (general, completed); verifier claude-sonnet-5 (primary, completed). Orchestrator: openai/gpt-5.6-sol, reasoning=high, orchestration-only.
🔴 2 blocking
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/kotlin-sdk/sdk/build.gradle.kts`:
- [BLOCKING] packages/kotlin-sdk/sdk/build.gradle.kts:58-62: Release publication is missing a javadoc jar — real Maven Central deploys will be rejected
This delta turns on a real Maven Central / Sonatype deploy path (`jreleaser { deploy { maven { mavenCentral { ... } nexus2 { ... } } } }`, lines 245-273), meaning `:sdk:jreleaserDeploy` is now expected to actually push release and snapshot artifacts to Central. `android.publishing.singleVariant("release")` (lines 58-62) only calls `withSourcesJar()` — it never calls `withJavadocJar()`. Central's Publisher Portal requires a javadoc jar alongside the sources jar for every non-POM component (https://central.sonatype.org/publish/requirements/), and the snapshot deployer even sets `applyMavenCentralRules(true)` (line 263), which makes JReleaser enforce that rule before upload. The gap in `singleVariant` predates this commit, but it was inert until this delta made the deploy path real — `publishToMavenLocal` never exercises it, so this wasn't caught by local verification. Without this, the first real release or snapshot deploy fails validation.
- [BLOCKING] packages/kotlin-sdk/sdk/build.gradle.kts:178-183: Documented in-memory signing env vars are never wired to a signatory
The comment directly above this block (lines 173-177) says signing keys can come from either the legacy `signing.keyId` / `signing.password` / `signing.secretKeyRingFile` properties or the in-memory `ORG_GRADLE_PROJECT_signingKey` / `signingPassword` pair. The latter environment variables only populate Gradle project properties named `signingKey` and `signingPassword`; the signing plugin does not consume them automatically, and this block never calls `useInMemoryPgpKeys`. Therefore a CI release following the documented in-memory-key path fails in `signReleasePublication` before staging anything. Explicitly read those properties and pass them to `useInMemoryPgpKeys(signingKey, signingPassword)`; the separate legacy keyring-file path can continue to use Gradle's standard signing properties.
| publishing { | ||
| singleVariant("release") { | ||
| withSourcesJar() | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Release publication is missing a javadoc jar — real Maven Central deploys will be rejected
This delta turns on a real Maven Central / Sonatype deploy path (jreleaser { deploy { maven { mavenCentral { ... } nexus2 { ... } } } }, lines 245-273), meaning :sdk:jreleaserDeploy is now expected to actually push release and snapshot artifacts to Central. android.publishing.singleVariant("release") (lines 58-62) only calls withSourcesJar() — it never calls withJavadocJar(). Central's Publisher Portal requires a javadoc jar alongside the sources jar for every non-POM component (https://central.sonatype.org/publish/requirements/), and the snapshot deployer even sets applyMavenCentralRules(true) (line 263), which makes JReleaser enforce that rule before upload. The gap in singleVariant predates this commit, but it was inert until this delta made the deploy path real — publishToMavenLocal never exercises it, so this wasn't caught by local verification. Without this, the first real release or snapshot deploy fails validation.
| publishing { | |
| singleVariant("release") { | |
| withSourcesJar() | |
| } | |
| } | |
| publishing { | |
| singleVariant("release") { | |
| withSourcesJar() | |
| withJavadocJar() | |
| } | |
| } |
source: ['sonnet5', 'codex']
There was a problem hiding this comment.
Resolved in fcaa91f — Release publication is missing a javadoc jar — real Maven Central deploys will be rejected no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| signing { | ||
| setRequired(Callable { | ||
| gradle.taskGraph.allTasks.any { it is PublishToMavenRepository } | ||
| }) | ||
| sign(publishing.publications["release"]) | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Documented in-memory signing env vars are never wired to a signatory
The comment directly above this block (lines 173-177) says signing keys can come from either the legacy signing.keyId / signing.password / signing.secretKeyRingFile properties or the in-memory ORG_GRADLE_PROJECT_signingKey / signingPassword pair. The latter environment variables only populate Gradle project properties named signingKey and signingPassword; the signing plugin does not consume them automatically, and this block never calls useInMemoryPgpKeys. Therefore a CI release following the documented in-memory-key path fails in signReleasePublication before staging anything. Explicitly read those properties and pass them to useInMemoryPgpKeys(signingKey, signingPassword); the separate legacy keyring-file path can continue to use Gradle's standard signing properties.
| signing { | |
| setRequired(Callable { | |
| gradle.taskGraph.allTasks.any { it is PublishToMavenRepository } | |
| }) | |
| sign(publishing.publications["release"]) | |
| } | |
| signing { | |
| val signingKey = project.findProperty("signingKey") as String? | |
| val signingPassword = project.findProperty("signingPassword") as String? | |
| if (signingKey != null) { | |
| useInMemoryPgpKeys(signingKey, signingPassword) | |
| } | |
| setRequired(Callable { | |
| gradle.taskGraph.allTasks.any { it is PublishToMavenRepository } | |
| }) | |
| sign(publishing.publications["release"]) | |
| } |
source: ['codex']
There was a problem hiding this comment.
Resolved in fcaa91f — Documented in-memory signing env vars are never wired to a signatory no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
|
Done in Added a jreleaser deploy block mirroring dashj/core/build.gradle — Validated: |
…; wire in-memory signing keys Addresses the latest dashpay#4045 review round. - Missing javadoc jar (blocking 3acc85ab85d4): singleVariant("release") now calls withJavadocJar() in addition to withSourcesJar(). Maven Central's Publisher Portal rejects any non-POM component without a javadoc jar, and the snapshot deployer's applyMavenCentralRules(true) enforces it before upload. AGP emits a Kotlin-appropriate javadoc jar — no Dokka dependency added. - JNI-less AAR guard (blocking 8b899bc2e5e8): -PallowMissingJni previously downgraded verifyJniLibsPresent to a warning for EVERY publish task, so it could push a nativeless AAR to a remote repo / jreleaser's staging dir. Split into verifyJniLibsForRemotePublish (ALWAYS hard-fails; flag not honored — wired to every PublishToMavenRepository and to jreleaserDeploy/Upload/Release) and verifyJniLibsForLocalPublish (honors the flag; wired to PublishToMavenLocal). Verified end-to-end: remote task fails with -PallowMissingJni; local warns. - In-memory signing keys (blocking d251acfba63b, from last round): the signing block documented ORG_GRADLE_PROJECT_signingKey/signingPassword but never called useInMemoryPgpKeys, so a CI release on that path failed in signReleasePublication. Now reads the signingKey/signingPassword properties and calls useInMemoryPgpKeys when present; absent (local dev) it's skipped and required=false needs no key. - kotlinx-serialization-json scope (suggestion 96b7b2a236ff): Sdk.MasternodeEntry is a wire DTO touched only by a private envelope and internal parse — never returned by a public function. Marked it `internal` so its generated serializer isn't public API; the dependency correctly stays `implementation` (cleaner than promoting it to `api`, the reviewer's own preferred fix). Validated: :sdk:publishToMavenLocal lands sources + javadoc jars under ~/.m2/repository/org/dashj/; :sdk unit suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-Forward Prior Findings
None — all three prior findings (missing javadoc jar, unwired in-memory signing keys, leaked public MasternodeEntry serializer) are verified FIXED at fcaa91f.
New Findings In Latest Delta
One nitpick: the module has zero .java sources, so AGP's generated javadoc jar via withJavadocJar() will satisfy Central's structural requirement but contain no real API documentation, despite the added comment calling it "Kotlin-appropriate." This doesn't block a Central upload since only artifact presence is validated.
Source (experiment sonnet-primary-opus-quarter-sample-20260710, cohort sonnet_primary, bucket 3): reviewers codex/general=gpt-5.6-sol(completed); sonnet5/general=claude-sonnet-5(completed); verifier=verifier-sonnet5-4045-1783826342=claude-sonnet-5; orchestrator=openai/gpt-5.6-sol reasoning=high (orchestration-only, not a reviewer/verifier).
💬 1 nitpick(s)
| // Maven Central's Publisher Portal rejects any non-POM component that | ||
| // lacks a javadoc jar, and the snapshot deployer sets | ||
| // applyMavenCentralRules(true) which enforces it before upload | ||
| // (dashpay/platform#4045). AGP emits a (Kotlin-appropriate) javadoc jar | ||
| // here — no Dokka dependency required. | ||
| withJavadocJar() |
There was a problem hiding this comment.
💬 Nitpick: Generated javadoc jar will be effectively empty — comment overstates what AGP produces
The module (packages/kotlin-sdk/sdk/src/main) contains zero .java source files — confirmed by search, it is pure Kotlin. AGP’s withJavadocJar() wires up the standard Javadoc Gradle task, which only processes Java sources; it does not understand Kotlin and will not document any of the SDK’s public Kotlin API. The comment at lines 64-65 (“AGP emits a (Kotlin-appropriate) javadoc jar here — no Dokka dependency required”) overstates this: the produced jar satisfies Central’s structural requirement (a -javadoc.jar artifact must exist) but contains no meaningful API documentation. This does not block a real Central upload because only artifact presence is validated, not content — flagging as a comment-accuracy nitpick in case real API docs via Dokka are wanted later.
source: [sonnet5-general]
|
Round-2 blockers addressed (verified against the current branch; validated with a local publish under a throwaway version):
|
…al release One-page post-merge runbook now that hosting is settled on Maven Central under org.dashj (dashpay#4045): prerequisites (Central Portal token for org.dashj, GPG key, NDK/cargo-ndk toolchain), the env-var-only credential wiring (ORG_GRADLE_PROJECT_signingKey/Password for the staging publish, JRELEASER_MAVENCENTRAL_SONATYPE_* / JRELEASER_NEXUS2_SNAPSHOTS_* / JRELEASER_GPG_* for the deploy), the three release commands in order (build_android.sh --verify -> staged publish -> jreleaserDeploy) with -PsdkVersion for real releases (default stays 0.1.0-SNAPSHOT), what the verifyJniLibsForRemotePublish guard refuses and why, and the consumer coordinate org.dashj:dash-sdk-android:<version>. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Hosting is now settled: Maven Central under Post-merge, the first release is:
@HashEngineering since the |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Carried-forward prior findings: none; the prior Javadoc nitpick is outdated because the configured AGP 8.9 API explicitly generates documentation from Java and Kotlin sources. New latest-delta findings: the release runbook has one blocking issue because it deploys a persistent staging repository without cleaning stale versions first. No additional cumulative findings were identified.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed_before_session_creation),gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/kotlin-sdk/PUBLISHING.md`:
- [BLOCKING] packages/kotlin-sdk/PUBLISHING.md:49-54: Clean the staging repository before assembling a release
The runbook publishes each requested version into the fixed `sdk/build/staging-deploy` Maven repository and then gives that entire directory to both configured JReleaser deployers. Maven publication adds or replaces files for the current coordinates but does not remove other version directories, and JReleaser explicitly recommends deploying from a clean state. Reusing this sequence after an earlier snapshot or release staging run can therefore bundle stale versions with the intended release, causing duplicate-version or validation failures and potentially uploading unintended artifacts. Run `:sdk:clean` before staging; it removes the configured build directory while preserving the JNI libraries under `src/main/jniLibs`.
| # 2. Stage the signed release AAR + sources/javadoc jars into sdk/build/staging-deploy. | ||
| ./gradlew :sdk:publishReleasePublicationToStagingRepository -PsdkVersion=0.1.0 | ||
|
|
||
| # 3. Upload the staged artifacts. Release versions -> Maven Central; | ||
| # -SNAPSHOT versions -> Sonatype snapshots. | ||
| ./gradlew :sdk:jreleaserDeploy -PsdkVersion=0.1.0 |
There was a problem hiding this comment.
🔴 Blocking: Clean the staging repository before assembling a release
The runbook publishes each requested version into the fixed sdk/build/staging-deploy Maven repository and then gives that entire directory to both configured JReleaser deployers. Maven publication adds or replaces files for the current coordinates but does not remove other version directories, and JReleaser explicitly recommends deploying from a clean state. Reusing this sequence after an earlier snapshot or release staging run can therefore bundle stale versions with the intended release, causing duplicate-version or validation failures and potentially uploading unintended artifacts. Run :sdk:clean before staging; it removes the configured build directory while preserving the JNI libraries under src/main/jniLibs.
| # 2. Stage the signed release AAR + sources/javadoc jars into sdk/build/staging-deploy. | |
| ./gradlew :sdk:publishReleasePublicationToStagingRepository -PsdkVersion=0.1.0 | |
| # 3. Upload the staged artifacts. Release versions -> Maven Central; | |
| # -SNAPSHOT versions -> Sonatype snapshots. | |
| ./gradlew :sdk:jreleaserDeploy -PsdkVersion=0.1.0 | |
| # 2. Clear previously staged versions, then stage the signed release AAR + sources/javadoc jars. | |
| ./gradlew :sdk:clean | |
| ./gradlew :sdk:publishReleasePublicationToStagingRepository -PsdkVersion=0.1.0 | |
| # 3. Upload the staged artifacts. Release versions -> Maven Central; | |
| # -SNAPSHOT versions -> Sonatype snapshots. | |
| ./gradlew :sdk:jreleaserDeploy -PsdkVersion=0.1.0 |
source: ['codex']
There was a problem hiding this comment.
Fixed in the latest push: a cleanStagingDeploy Delete task now wipes sdk/build/staging-deploy before every *ToStagingRepository publish (wired via tasks.withType<PublishToMavenRepository>), so JReleaser always deploys from a clean staging state regardless of prior snapshot/release runs. Verified in the task graph with --dry-run; PUBLISHING.md updated to note the automatic wipe.
maven-publish adds/replaces the current coordinates but never removes other versions' directories from sdk/build/staging-deploy, so a release deploy run after an earlier snapshot/release staging could hand JReleaser stale coordinates. cleanStagingDeploy now runs before every *ToStagingRepository publish (verified in the task graph via --dry-run); PUBLISHING.md notes it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
Adds
maven-publishto the Kotlin SDK library so consumers can depend on a Maven coordinate instead of a GitHub-release AAR.org.dashfoundation:dash-sdk-android(version via-PsdkVersion=x.y.z, defaults to0.1.0-SNAPSHOT)apideps carried in the POM)./gradlew :sdk:publishToMavenLocalverified end-to-end: with../build_android.shrun first, the published AAR packageslibdash_sdk_jni.sofor arm64-v8a and x86_64, and the Dash Android wallet consumes it from mavenLocal alongside dashj with no class conflicts (see Remove CoinJoin mixing; begin Kotlin SDK migration (Phases 1, 2, 0 prep + Phase 3 start) dash-wallet#1507)repositories {}block is intentionally left out pending a hosting decision (Maven Central vs GitHub Packages)Context: Phase 0 of the Android wallet's dashj → Kotlin SDK migration.
🤖 Generated with Claude Code
Summary by CodeRabbit