Repository navigation
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 134 files, which is 34 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (134)
You can disable this status message by setting the 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 |
ttypic
force-pushed
the
integration/v2
branch
from
September 24, 2026 10:19
bcb7606 to
24bd01a
Compare
The stateless client and the types reachable from it were named after the Ably REST API they talk to rather than the transport they use, which reads inconsistently beside Realtime and beside the already HTTP-named pubsub/server aliases. Rename REST -> HTTPClient, RESTChannels -> HTTPChannels, RESTChannel -> HTTPChannel, RESTPresence -> HTTPPresence, RESTRequest -> HTTPRequest, RESTPaginatedItems -> HTTPPaginatedItems, NewREST -> NewHTTPClient, WithRESTHost -> WithHTTPHost and WithIdempotentRESTPublishing -> WithIdempotentHTTPPublishing. This is a breaking change: the old names are removed outright rather than kept as deprecated aliases, so a /v2 module path is required before release. Stats.REST keeps its name, because it mirrors the "rest" field of the stats payload rather than naming a client type. - Extend pubsub/server with aliases for the types reachable through HTTPClient, so its users never name an ably.* type directly - Unexport ablytest's stdlib *http.Client helper, freeing the NewHTTPClient name for the sandbox helper that was ablytest.NewREST - Document the old-to-new mapping in UPDATING.md Tests and examples follow the new names. No behaviour changes, so no new coverage is added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The SDK was entered through github.com/ably/ably-go/ably, which left the
pubsub packages as a thin veneer over an API users could equally import
directly, and gave them no way to see which half of it applied to them.
Move the implementation to internal/ably, where nothing outside this
module can import it, and have pubsub/server and pubsub/device re-export
the part of it their users need. The re-exports are type aliases, so the
types stay identical across both packages.
Generate the re-exports rather than listing them, since a hand-written
mirror of ~200 declarations drifts the first time internal/ably changes.
internal/cmd/genapi aliases every exported type by default — an
incomplete walk of the API can then only over-expose, never silently
drop — and emits a function or constant once every ably type in its
signature is aliased too. pubsub/device additionally drops what only an
HTTPClient can reach, which a device client has no way to obtain.
Three consequences worth knowing, all recorded in UPDATING.md:
- LiveObjects types take an Objects prefix, because objects.Message and
ably.Message cannot share a package
- WithDial is not re-exported: its signature names an unexported type,
so no external caller could ever supply an argument to it
- internal/ably/http_{client,channel,presence}.go are the former rest_*
files, renamed to match the types they hold
api_test.go in each package exercises a broad slice of the re-exported
surface, so a symbol dropped from the generated set fails here rather
than in a user's build. genapi_test.go fails when the generated files
no longer match the generator, alongside the existing ci-generate.sh
check that go generate leaves a clean tree.
Examples now use pubsub/server, and examples/rest is examples/http.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pubsub directory added a path segment that said nothing the module path did not already say, and the SDK is no longer an ably-go: after the previous commit it exposes only the Pub/Sub entry points. Rename the module to github.com/ably/ably-pubsub-go and lift the two entry-point packages to the repository root, so that they are imported as .../ably-pubsub-go/server and .../ably-pubsub-go/device. The module rename stands in for the /v2 path suffix this release would otherwise need, so there is no ably-go/v2. genapi needed more than a path rewrite: its repoRoot walked up looking for the literal old module line in go.mod, so `go generate` failed until it was pointed at the new path. Both that lookup and its error message now come from one modulePath constant. The repository has not moved yet, so github.com/ably/ably-go URLs are left alone and the README says the new path does not resolve until it does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
server and device named the package after its directory, so the same call read as server.NewRealtimeClient in one and device.NewClient in the other. Which side an application runs on is already settled by the import path it chose; repeating it on every call added nothing. Name both packages pubsub. Only the import path now distinguishes them, and moving an application between the two is a one-line change to that path rather than a rewrite of every call site. Note that a bare import of either path binds pubsub, not server or device, since the identifier comes from the package clause and not the last path element; the package docs and the migration guide both say so, and no import aliases the name back to its directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit named both entry-point packages pubsub while leaving them in server and device, so the identifier an import bound differed from the last element of its path. Go tooling and readers expect the two to match, and the docs had to warn about it. Move the packages one level down, to .../server/pubsub and .../device/pubsub. The server or device segment still says which side the code runs on, and the package name now follows the path. The genapi targets, go generate hints, examples and docs follow the new paths, and the warning about the mismatch is gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ttypic
force-pushed
the
integration/renaming
branch
from
September 24, 2026 10:27
17f6df2 to
82657ad
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This pull request introduces major structural and naming adjustments to the Pub/Sub SDK. It includes the following changes:
deviceandservertopubsub, streamlining usage and reducing redundancy in import paths.ably-pubsub-goand flattened entry points, simplifying directory structure and imports.internal/ablyto prevent direct imports and re-exported necessary symbols viapubsub/serverandpubsub/device.RESTAPI symbols toHTTP*for consistency and replaced all related examples and tests./devicefor end-user device-specific clients./serverfor server-side clients with updated headers for classification.