Skip to content

rename spaces get-all to get to align with the rest of the cli#192

Merged
umair-ably merged 3 commits intomainfrom
spaces-get
Mar 26, 2026
Merged

rename spaces get-all to get to align with the rest of the cli#192
umair-ably merged 3 commits intomainfrom
spaces-get

Conversation

@umair-ably
Copy link
Collaborator

@umair-ably umair-ably commented Mar 25, 2026

This PR renames all spaces * get-all subcommands to spaces * get for consistency with the rest of the CLI. For spaces locks, the two separate commands (get and get-all) are merged into a single get command where LOCKID is now optional — omitting it returns all locks, providing it returns a single lock.

@vercel
Copy link

vercel bot commented Mar 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cli-web-cli Ready Ready Preview, Comment Mar 26, 2026 1:30pm

Request Review

@claude-code-ably-assistant
Copy link

Walkthrough

This PR renames all spaces * get-all subcommands to spaces * get for consistency with the rest of the CLI. For spaces locks, the two separate commands (get and get-all) are merged into a single get command where LOCKID is now optional — omitting it returns all locks, providing it returns a single lock.

Changes

Area Files Summary
Commands src/commands/spaces/cursors/get-all.tsget.ts Renamed file and class; updated component string from cursorGetAll to cursorGet
Commands src/commands/spaces/locations/get-all.ts → (deleted), get.ts (new) Deleted old file; new get.ts is functionally equivalent with updated examples
Commands src/commands/spaces/locks/get.ts LOCKID made optional; now dispatches to locks.get() or locks.getAll() based on presence of arg; get-all.ts deleted
Commands src/commands/spaces/members/get-all.tsget.ts Renamed; also adds missing --client-id flag
Commands src/commands/spaces/cursors.ts, cursors/index.ts, locations.ts, locations/index.ts, members/index.ts, locks/index.ts Updated example strings from get-all to get
Base src/base-command.ts Added _suppressSdkErrorLogs flag to silence SDK error-level logs during teardown (e.g. Spaces cursors 80017 on disconnect)
Utils src/utils/spaces-output.ts formatLockBlock updated to accept optional indent option for consistent indentation in multi-lock output
Tests test/unit/commands/spaces/cursors/get-all.test.tsget.test.ts Renamed; error test tightened (now expects error instead of accepting any stdout)
Tests test/unit/commands/spaces/locations/get-all.test.tsget.test.ts Renamed; no logic changes
Tests test/unit/commands/spaces/members/get-all.test.tsget.test.ts Renamed; no logic changes
Tests test/unit/commands/spaces/locks/get-all.test.ts Deleted; coverage migrated into get.test.ts
Tests test/unit/commands/spaces/locks/get.test.ts Expanded with all-locks scenarios; standardArgValidationTests replaces manual arg checks; LOCKID is now optional so space-only arg is valid
Tests test/e2e/spaces/spaces-e2e.test.ts Updated get-allget in e2e cursor test
Docs README.md Regenerated to reflect renamed commands and updated locks get signature

Review Notes

  • Breaking change: Any scripts or tooling using ably spaces cursors get-all, ably spaces locations get-all, or ably spaces members get-all will break. The ably spaces locks get-all command is also removed. No deprecation shim or alias is provided.
  • Behavioral change in locks get: Previously LOCKID was required; now it is optional. This is a loosening of the argument contract, which is backwards-compatible for callers passing a lock ID but removes the guard for callers that omitted it accidentally.
  • members get gains --client-id: The old get-all command lacked this flag; the new get command includes it. Low risk, but worth confirming the flag has no unintended side-effect on the passive (non-entering) read path.
  • _suppressSdkErrorLogs in base command: This is a shared mutable flag set by SpacesBaseCommand.finally(). Reviewers should verify it's only set after run() completes and cannot suppress errors that should be visible during normal operation.
  • No migration guide or changelog entry is included — consider whether one is needed for consumers of this CLI.

Ably capabilities are operation-based, not clientId-based, so client
identity is irrelevant for pure read queries. Removed clientIdFlag from
spaces members/locations/cursors/locks get and rooms occupancy get.
Updated docs and skills to clarify when --client-id should be used.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Renames Spaces CLI subcommands from get-all to get for consistency, and merges Spaces locks “get” + “get-all” into a single get command with optional LOCKID.

Changes:

  • Renamed Spaces cursors/locations/members get-all to get across command implementations, docs, and unit/E2E tests.
  • Merged spaces locks get-all into spaces locks get SPACE_NAME [LOCKID], adding “get all locks” behavior when LOCKID is omitted.
  • Updated output formatting (lock block indentation) and adjusted flag guidance/usage (notably removing client-id from read-only commands) and suppressed teardown SDK error logs.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/unit/commands/spaces/members/get.test.ts Updates unit tests to use spaces:members:get instead of get-all.
test/unit/commands/spaces/locks/get.test.ts Expands tests for merged locks get behavior (single vs all) and updates arg validation expectations.
test/unit/commands/spaces/locks/get-all.test.ts Removes tests for deleted spaces:locks:get-all command.
test/unit/commands/spaces/locations/get.test.ts Updates unit tests to use spaces:locations:get.
test/unit/commands/spaces/cursors/get.test.ts Updates unit tests to use spaces:cursors:get and aligns error expectations.
test/e2e/spaces/spaces-e2e.test.ts Updates E2E invocation from cursors get-all to cursors get.
src/utils/spaces-output.ts Adds indentation support to formatLockBlock for list output formatting.
src/spaces-base-command.ts Suppresses SDK error logs during teardown to avoid noisy/benign cleanup errors.
src/flags.ts Clarifies when client-id is applicable; treats read-only queries as not needing it.
src/commands/spaces/members/index.ts Updates example from get-all to get.
src/commands/spaces/members/get.ts Renames implementation to SpacesMembersGet and updates telemetry/error keys.
src/commands/spaces/members.ts Updates example from get-all to get.
src/commands/spaces/locks/index.ts Updates example from get-all to get (no LOCKID).
src/commands/spaces/locks/get.ts Merges “get single” + “get all”, makes LOCKID optional, updates formatting and initialization behavior.
src/commands/spaces/locks/get-all.ts Removes command implementation for spaces locks get-all.
src/commands/spaces/locks.ts Updates example from get-all to get.
src/commands/spaces/locations/index.ts Updates example from get-all to get.
src/commands/spaces/locations/get.ts Adds new spaces locations get command implementation (replacing get-all).
src/commands/spaces/locations/get-all.ts Removes command implementation for spaces locations get-all.
src/commands/spaces/locations.ts Updates example from get-all to get.
src/commands/spaces/cursors/index.ts Updates example from get-all to get.
src/commands/spaces/cursors/get.ts Renames implementation to SpacesCursorsGet and updates telemetry/error keys.
src/commands/spaces/cursors.ts Updates example from get-all to get.
src/commands/rooms/occupancy/get.ts Removes client-id flag from read-only occupancy get.
src/base-command.ts Adds log-handler suppression toggle for SDK error logs during teardown.
README.md Regenerates CLI docs to reflect renamed commands and updated flags/usage.
.claude/skills/ably-review/SKILL.md Updates internal review guidance around clientIdFlag usage for reads vs writes.
.claude/skills/ably-new-command/references/patterns.md Updates internal patterns guidance for “Get” commands and clientIdFlag.
.claude/skills/ably-new-command/SKILL.md Updates internal guidance for when to include clientIdFlag.
.claude/skills/ably-codebase-review/SKILL.md Updates internal review guidance for clientIdFlag usage.
Comments suppressed due to low confidence (2)

src/commands/spaces/cursors/get.ts:37

  • The old spaces:cursors:get-all command appears to be removed/renamed without providing an oclif alias. This is a breaking CLI change for existing scripts; consider adding static override aliases = ["spaces:cursors:get-all"] (and potentially a deprecation warning) so get-all continues to work while users migrate to get.
    src/commands/spaces/members/get.ts:37
  • The old spaces:members:get-all command appears to be removed/renamed without providing an oclif alias. This is a breaking CLI change for existing scripts; consider adding static override aliases = ["spaces:members:get-all"] (and optionally a deprecation warning) so get-all continues to work while users migrate to get.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@sacOO7 sacOO7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
PS. copilot comments doesn't seem important as such, but you can check

@umair-ably umair-ably merged commit 0560abc into main Mar 26, 2026
13 checks passed
@umair-ably umair-ably deleted the spaces-get branch March 26, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants