[ECO-5383][LiveObjects] Enable maven publish#1143
Conversation
|
Warning Rate limit exceeded@sacOO7 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 54 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 (1)
WalkthroughThe changes introduce instructions for publishing to a local Maven repository in the documentation, update the build configuration to skip signing when publishing locally, and add the Maven Publish plugin to the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Gradle
participant Local Maven Repo
Developer->>Gradle: Run ./gradlew publishToMavenLocal
Gradle->>Gradle: Detects publishToMavenLocal task
Gradle-->>Gradle: Skips signing step
Gradle->>Local Maven Repo: Publishes artifact locally
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Assessment against linked issues
Poem
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
sacOO7
force-pushed
the
fix/local-maven-publish
branch
from
f96e5d1 to
3c4413e
Compare
sacOO7
force-pushed
the
fix/local-maven-publish
branch
from
3c4413e to
a2ef7b9
Compare
sacOO7
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
build.gradle.kts (1)
33-40: Harden local-publish detection and add a property-based opt-out for signingUse endsWith("ToMavenLocal") to precisely match publish-to-local tasks and support a -PskipSigning override. This is also configuration-cache friendly via providers.
- // Check if we're running a local publish task - val isLocalPublish = gradle.startParameter.taskNames.any { - it.contains("publishToMavenLocal") || it.contains("ToMavenLocal") - } - - if (!isLocalPublish) { - signAllPublications() - } + // Skip signing when publishing to Maven Local or when explicitly requested via -PskipSigning=true + val skipSigning = gradle.startParameter.taskNames.any { task -> + task.endsWith("ToMavenLocal") + } || providers.gradleProperty("skipSigning") + .map { it.toBoolean() } + .orElse(false) + .get() + + if (!skipSigning) { + signAllPublications() + }CONTRIBUTING.md (3)
50-63: Clarify module-specific publish command and repository resolution order
- Consider adding the module-scoped task to publish only LiveObjects.
- Note that mavenLocal() should typically precede mavenCentral() so locally built artifacts take precedence.
To publish the library to your local Maven repository, you can use the following command: -```bash -./gradlew publishToMavenLocal -``` + ./gradlew publishToMavenLocal + +Alternatively, to publish only the LiveObjects module: + + ./gradlew :live-objects:publishToMavenLocal @@ -To use the locally published library in your project, you can add the following dependency in your `build.gradle` file: +To use the locally published library in your project, add the following repository in your `build.gradle` file (place `mavenLocal()` before `mavenCentral()` for resolution precedence): -```groovy -repositories { - mavenLocal() -} -``` + repositories { + mavenLocal() + mavenCentral() + }
53-55: Fix markdownlint MD046: use indented code blocks (not fenced)Switch the fenced block to an indented block to match the rest of this doc and satisfy the linter.
-```bash -./gradlew publishToMavenLocal -``` + ./gradlew publishToMavenLocal
56-62: Fix MD046 and wording: it’s a “repository”, not a “dependency”Align with project style (indented blocks) and correct wording.
-To use the locally published library in your project, you can add the following dependency in your `build.gradle` file: +To use the locally published library in your project, add the following repository in your `build.gradle` file: -```groovy -repositories { - mavenLocal() -} -``` + repositories { + mavenLocal() + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CONTRIBUTING.md(1 hunks)build.gradle.kts(1 hunks)live-objects/build.gradle.kts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:48-61
Timestamp: 2025-06-03T09:15:18.827Z
Learning: User sacOO7 prefers simple test utilities without extensive error handling, believing tests should fail fast if incorrect field/method names are used rather than having defensive programming.
📚 Learning: 2025-08-07T07:17:33.340Z
Learnt from: sacOO7
PR: ably/ably-java#1137
File: live-objects/src/test/kotlin/io/ably/lib/objects/unit/type/livemap/LiveMapManagerTest.kt:6-6
Timestamp: 2025-08-07T07:17:33.340Z
Learning: In the ably-java LiveObjects test code, there are extension properties defined in TestHelpers.kt that provide access to private fields of classes for testing purposes. For example, `internal var DefaultLiveMap.LiveMapManager: LiveMapManager` allows tests to access the private `liveMapManager` field. These extension imports (like `import io.ably.lib.objects.unit.LiveMapManager`) should not be removed as they are necessary for test functionality and are not conflicting imports.
Applied to files:
live-objects/build.gradle.kts
📚 Learning: 2025-06-05T10:27:53.946Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: gradle/libs.versions.toml:24-26
Timestamp: 2025-06-05T10:27:53.946Z
Learning: The ably-java project prefers to use the latest available versions of testing dependencies (including pre-release versions) when they contain relevant bug fixes, rather than sticking strictly to stable releases.
Applied to files:
CONTRIBUTING.md
🪛 markdownlint-cli2 (0.17.2)
CONTRIBUTING.md
53-53: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
58-58: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🔇 Additional comments (1)
live-objects/build.gradle.kts (1)
6-6: LGTM: Publishing plugin applied at module levelThis enables publishToMavenLocal for live-objects as intended. Ensure group/version are set (likely via root) for consistent coordinates.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
CONTRIBUTING.md (2)
62-62: Typo: it’s a repositories block, not a dependencyThis sentence refers to adding a repository, not a dependency.
-- To use the locally published library in your project, you can add the following dependency in your `build.gradle` file: +- To use the locally published library in your project, add the following to the `repositories` block in your build script:
69-71: Add troubleshooting tips for mavenLocal precedence and stale artifactsPractical tips help avoid common pitfalls when switching resolution to mavenLocal.
- Note - Place `mavenLocal()` before `mavenCentral()` for resolution precedence. +Troubleshooting: +- If your build still pulls from Maven Central, run with `--refresh-dependencies`. +- If a stale local artifact is being used, delete it from `~/.m2/repository/io/ably/...` or bump the library version (for example, append `-SNAPSHOT`) before publishing. +- If you changed other modules that `live-objects` depends on, publish them too (or run `./gradlew publishToMavenLocal` at the repo root).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:48-61
Timestamp: 2025-06-03T09:15:18.827Z
Learning: User sacOO7 prefers simple test utilities without extensive error handling, believing tests should fail fast if incorrect field/method names are used rather than having defensive programming.
📚 Learning: 2025-06-05T10:27:53.946Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: gradle/libs.versions.toml:24-26
Timestamp: 2025-06-05T10:27:53.946Z
Learning: The ably-java project prefers to use the latest available versions of testing dependencies (including pre-release versions) when they contain relevant bug fixes, rather than sticking strictly to stable releases.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-06-06T09:28:12.298Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/setup/IntegrationTest.kt:87-89
Timestamp: 2025-06-06T09:28:12.298Z
Learning: The Sandbox.kt file in ably-java live-objects module already has comprehensive HTTP retry mechanism using HttpRequestRetry with 5 retries, exponential backoff, and automatic retry on non-success responses and timeout exceptions.
Applied to files:
CONTRIBUTING.md
🪛 markdownlint-cli2 (0.17.2)
CONTRIBUTING.md
53-53: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
58-58: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
64-64: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
⏰ 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). (11)
- GitHub Check: build
- GitHub Check: check-rest-okhttp
- GitHub Check: check-liveobjects
- GitHub Check: check-realtime
- GitHub Check: check-realtime-okhttp
- GitHub Check: check-rest
- GitHub Check: check (24)
- GitHub Check: check (21)
- GitHub Check: check (29)
- GitHub Check: check (19)
- GitHub Check: check
🔇 Additional comments (1)
CONTRIBUTING.md (1)
50-60: No extra flag needed—signing is auto-skipped for local publishes
The root build.gradle.kts already checks forpublishToMavenLocalingradle.startParameter.taskNames(isLocalPublish) and wraps thesigning { … }block insideif (!isLocalPublish), so running./gradlew publishToMavenLocalbypasses signing without any additional properties or flags.
- File: build.gradle.kts, lines 33–38
No further changes required.
sacOO7
force-pushed
the
fix/local-maven-publish
branch
from
d6b0632 to
9c120a0
Compare
sacOO7
force-pushed
the
fix/local-maven-publish
branch
from
9c120a0 to
c75fb05
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
CONTRIBUTING.md (1)
63-67: Fix markdownlint MD046: use indented code block (not fenced) for consistencyReplace the fenced block with an indented code block to match the doc’s prevailing style and keep lint green.
-```groovy -repositories { - mavenLocal() -} -``` + repositories { + mavenLocal() + }
🧹 Nitpick comments (2)
CONTRIBUTING.md (2)
61-61: Wording: “dependency” → “repository”This line introduces a repositories block, not a dependency. Update for accuracy.
-- To use the locally published library in your project, you can add the following dependency in your `build.gradle` file: +- To use the locally published library in your project, add the following repository in your `build.gradle` file:
50-59: Optional: Add a short note that signing is skipped for mavenLocalThis aligns the docs with the build change that conditionally skips signing for local publishing, avoiding confusion.
To publish the library to your local Maven repository, you can use the following command: ./gradlew publishToMavenLocal +Note: Publishing to mavenLocal skips signing (pre-configured in the build), so no signing setup is required for local testing. Alternatively, to publish only the specific (LiveObjects) module:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: sacOO7
PR: ably/ably-java#1106
File: live-objects/src/main/kotlin/io/ably/lib/objects/serialization/Serialization.kt:25-29
Timestamp: 2025-06-23T14:18:25.315Z
Learning: In the ably-java codebase, the DefaultLiveObjectSerializer class methods like writeMsgpackArray will have their type signatures updated to be non-nullable since the calling sites ensure objects are never null when passed to these methods.
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:48-61
Timestamp: 2025-06-03T09:15:18.827Z
Learning: User sacOO7 prefers simple test utilities without extensive error handling, believing tests should fail fast if incorrect field/method names are used rather than having defensive programming.
📚 Learning: 2025-08-07T07:17:33.340Z
Learnt from: sacOO7
PR: ably/ably-java#1137
File: live-objects/src/test/kotlin/io/ably/lib/objects/unit/type/livemap/LiveMapManagerTest.kt:6-6
Timestamp: 2025-08-07T07:17:33.340Z
Learning: In the ably-java LiveObjects test code, there are extension properties defined in TestHelpers.kt that provide access to private fields of classes for testing purposes. For example, `internal var DefaultLiveMap.LiveMapManager: LiveMapManager` allows tests to access the private `liveMapManager` field. These extension imports (like `import io.ably.lib.objects.unit.LiveMapManager`) should not be removed as they are necessary for test functionality and are not conflicting imports.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-06-05T10:27:53.946Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: gradle/libs.versions.toml:24-26
Timestamp: 2025-06-05T10:27:53.946Z
Learning: The ably-java project prefers to use the latest available versions of testing dependencies (including pre-release versions) when they contain relevant bug fixes, rather than sticking strictly to stable releases.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-06-06T09:28:12.298Z
Learnt from: sacOO7
PR: ably/ably-java#1095
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/setup/IntegrationTest.kt:87-89
Timestamp: 2025-06-06T09:28:12.298Z
Learning: The Sandbox.kt file in ably-java live-objects module already has comprehensive HTTP retry mechanism using HttpRequestRetry with 5 retries, exponential backoff, and automatic retry on non-success responses and timeout exceptions.
Applied to files:
CONTRIBUTING.md
🪛 markdownlint-cli2 (0.17.2)
CONTRIBUTING.md
63-63: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
⏰ 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). (11)
- GitHub Check: check (24)
- GitHub Check: check (29)
- GitHub Check: check (19)
- GitHub Check: check (21)
- GitHub Check: check-rest-okhttp
- GitHub Check: check-realtime
- GitHub Check: check
- GitHub Check: check-rest
- GitHub Check: check-liveobjects
- GitHub Check: check-realtime-okhttp
- GitHub Check: build
2 participants
Related to #1094
Summary by CodeRabbit
Documentation
Chores