Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Integration branch for io.ably.pubsub:* v2 packages #1245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Integration branch for io.ably.pubsub:* v2 packages #1245
Changes from all commits
b59f806acb8e2ed22cbf0486efbf35fd6c3bf00ff4File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MAJOR]
javadoc.ymlassumes the old per-repo IAM role — docs upload fails silently after the renameBefore running
ably/sdk-upload-actionon thissourcePath,javadoc.ymlassumesrole/ably-sdk-builds-ably-java(line 24). This repo is renamed toably-pubsub-javaunder the PubSub device/server split rollout (PDR-091b2), and the rollout explicitly warns that these per-repo OIDC/IAM bindings break on rename: the role's trust policy no longer matches the workflow's OIDC subject, so the docs upload starts failing silently on the next push to main. The IAM role (name and trust-policy repo binding) needs re-provisioning forably/ably-pubsub-java, and the ARN on line 24 updated, in the same pass as the rename.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MAJOR] Docs published only for the internal
core; the public doors get no docs siteThis uploads only
core/build/docs/javadoc, and thedevicemodule publishes an empty javadoc jar. Under the PubSub device/server split rollout (PDR-091b2),coreis the internal-only artifact andPubSubDevice/PubSubServerare the recommended public entry points — so as configured, the docs site documents the one artifact users must not depend on and omits the public API. Please generate and publish docs for thedeviceandservermodules (in addition to, or instead of,core).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CRITICAL]
release.yamlsilently no-ops after the repo renamerelease.yamlguards the release job withif: github.repository == 'ably/ably-java'(line 9), but this repository is nowably/ably-pubsub-java(assettings.gradle.kts'srootProject.namealready reflects, per the PubSub device/server split rollout, PDR-091b2). With the old name in the guard the condition is false, the job is skipped, and the workflow reports success having published nothing — releases silently stop. Update the guard toably/ably-pubsub-java, keeping the fork-protection intent, and sweep for any othergithub.repositoryguards. (Anchored on the added pre-flight step; the guard itself is on line 9, outside this PR's hunks.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: ably/ably-pubsub-java
Length of output: 5526
🤖 get_repo_knowledge executed:
get_repo_knowledge ably/ably-pubsub-java /tmp/coderabbit-repo-knowledge/ably-ably-pubsub-java-5645e28c/learningsLength of output: 2076
Delete the active Gradle settings file. The repository contains
settings.gradle.kts, notsettings.gradle. Update this instruction to referencesettings.gradle.kts; otherwise users will target a file that does not exist.🧰 Tools
🪛 LanguageTool
[style] ~163-~163: Consider using “incapable” to avoid wordiness.
Context: ... development of the core project, it is not capable of importing the multi-level ably-java ...
(NOT_ABLE_PREMIUM)
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MINOR] No migration guide or 2.0.0 CHANGELOG; README install/sample still uses the deprecated 1.x path
This new README note is a good start, but before release there's no migration path for
io.ably:ably-java1.x users: which coordinate to move to, which door to pick per side, and that the old constructors will throw once MAU-based pricing is live (the forcing function in the PubSub device/server split rollout, PDR-091b2). The install section and sample further down still use the now-deprecatednew AblyRealtime(...)without flagging it, the badges still resolve the legacyio.ably/ably-javaartifact, and CHANGELOG has no 2.0.0 entry. (Minor: the door signature in the table above readsclientBuilder(...)though the factory takes no args — the chain isclientBuilder().key(...).build().) Please add a migration section and a 2.0.0 CHANGELOG entry before this ships.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: ably/ably-pubsub-java
Length of output: 39826
🏁 Script executed:
Repository: ably/ably-pubsub-java
Length of output: 34564
🏁 Script executed:
Repository: ably/ably-pubsub-java
Length of output: 2520
Create the 1.x maintenance branch before merging 2.0. No dedicated 1.x maintenance branch is present among the remote heads, while the current build is
VERSION_NAME=2.0.0. Create and protect a branch that retains theio.ably:ably-javaandio.ably:ably-android1.x artifact definitions, or revise this support promise.🤖 Prompt for AI Agents
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MINOR] Lockstep pre-flight models the KMP
devicemodule as a singledevice:jarverifyReleaseArtifactsassertsio.ably.pubsub:device:jar, butdeviceis a Kotlin Multiplatform module that publishesdevice(metadata),device-jvmanddevice-android. The pre-flight therefore doesn't model the coordinates consumers actually resolve and wouldn't catch drift in the-jvm/-androidvariants. Please enumerate the real KMP publications inexpectedReleaseArtifacts, or document that the KMP variants are approximated.Uh oh!
There was an error while loading. Please reload this page.