READMEs: fix every sample that misstates the API - #507
Merged
Merged
Conversation
The Quick Start passes project_id: 12345, but the generated signature has no such keyword -- todos are addressed by todolist (POST /todolists/:todolist_id/todos.json) and TodosExtensions never overrides create. Running the documented call raises ArgumentError: unknown keyword: :project_id at argument binding, before any HTTP. Also: "45 account-scoped services" is stale since EverythingService landed; the generated set is 46.
github-actions
Bot
added
typescript
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9df3c594f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This is a documentation-only PR that corrects README code samples and a handful of in-source doc comments across all six SDKs (Go, TypeScript, Ruby, Swift, Kotlin, Python) so that every published snippet matches the real generated API surface, pagination behavior, and compile-time requirements. It fits the repo's "generated-from-spec" architecture: none of the changes touch runtime behavior — they only bring hand-maintained docs back in sync with the generated services. I verified each corrected signature, count, and behavioral claim against the generated code:
- Go:
Projects().Listreturns*ProjectListResult(iterateresult.Projects);Campfires().List/ListLinesandWebhooks().Listall require the trailing options arg;go.modisgo 1.26; the OTel fence'shooks =reassignment and addedprometheusimport are correct. - Ruby
todos.createtakestodolist_id:(noproject_id:); Kotlin/Swifttodos.get(todoId)match the generated single-arg signatures; Kotlin'swhenis now exhaustive over all 11BasecampExceptionsubclasses;consoleHooksemitsProjects.ListProjects;maxRetries/baseRetryDelayare indeed absent fromBasecampClientBuilder. - TypeScript
list()auto-paginates and returnsListResult<T>with.meta,maxItemsbounds it, the 10,000-page cap and@37signals/basecampexports offetchAllPages/paginateAllall check out. - Service count is 46 in each SDK (Swift/Kotlin/Ruby/Python file counts confirmed).
Changes:
- Fix incorrect method signatures/arguments in README + doc-comment samples (Ruby
todos.create, Go list calls, Kotlin/Swifttodos.get). - Correct behavioral/config documentation (TS auto-pagination +
ListResult, Kotlin exhaustive errorwhen/fixed retry settings/hook output, Go 1.26 and compiling fences). - Update the stale "45 services" → "46" across all six SDKs.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
One minor consistency gap worth noting for a follow-up (outside the changed lines, so not annotated inline): swift/Sources/Basecamp/BasecampError.swift:17 still shows Task.sleep(for: .seconds(seconds)) — the exact macOS 13-only API this PR replaced in swift/README.md — within the same doc-comment sample whose signature was corrected on line 10. Aligning it with the Task.sleep(nanoseconds:) form would fully deliver the PR's stated goal on the declared macOS 12 floor.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
go/README.md |
Iterate result.Projects, pass nil options to Campfires/Webhooks list calls, fix OTel reassignment + prometheus import, bump to Go 1.26 |
typescript/README.md |
Rewrite Pagination to document list() auto-pagination, ListResult/.meta, maxItems, and typed fetchAllPages/paginateAll |
ruby/README.md |
Remove invalid project_id: from todos.create; 45→46 services |
swift/README.md |
Fix todos.get(todoId:), replace macOS 13-only Task.sleep(for:) with nanoseconds form; 45→46 services |
swift/Sources/Basecamp/BasecampError.swift |
Fix phantom todos.get signature in doc comment |
swift/Sources/Basecamp/AccountClient.swift |
Fix phantom todos.get signature in doc comment |
swift/Sources/Basecamp/Pagination.swift |
Fix phantom todos.list signature in doc comment |
kotlin/README.md |
Fix todos.get, make error when exhaustive (+DiscoverySelection/DeviceFlow rows), mark retry settings fixed, correct hook output, 45→46 |
kotlin/.../BasecampException.kt |
Fix phantom todos.get signature in doc comment |
kotlin/.../BasecampClient.kt |
Fix phantom todos.get signature in doc comment |
python/README.md |
45→46 generated services |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeremy
force-pushed
the
docs/readme-example-fixes
branch
from
July 31, 2026 06:51
9df3c59 to
2937fd9
Compare
github-actions
Bot
removed
the
documentation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2937fd999c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
force-pushed
the
docs/readme-example-fixes
branch
from
July 31, 2026 06:56
2937fd9 to
665442f
Compare
Member
Author
|
Applied Copilot's catch: |
Five fences failed to compile against the SDK they document; each
defect reproduced as the exact compiler error offline (scratch module
with a replace directive, GOPROXY=off), and each corrected snippet
builds and vets clean:
- Both Quick Start programs range over the *ProjectListResult struct
("cannot range over projects"); iterate result.Projects, matching
the root README's correct form.
- Campfires().List / ListLines and Webhooks().List omit their options
parameter ("not enough arguments in call"); pass nil.
- The OTel fence redeclares hooks in the same scope ("no new variables
on left side of :="); plain assignment, and construct the client
after the hooks alternatives so the custom providers actually reach
it when copied as written.
- The chain-hooks fence uses prometheus.DefaultRegisterer without
importing prometheus ("undefined: prometheus"); add the import.
Also: "Requires Go 1.25 or later" contradicted go.mod's go 1.26.
The README's fragment conventions (undeclared ctx, use(), top-level
return err) are established style and stay as they are.
The Error Handling fence could not compile: todos.get has no projectId parameter (generated signature is get(todoId: Long)), and the when over the sealed BasecampException covers 9 of 11 subclasses -- a non-exhaustive when statement over a sealed subject is a compile error, and the SDK's own ErrorTest pins all 11 branches. Add the DiscoverySelection and DeviceFlow branches and their Error Types rows. Also: - "45 services" is stale since EverythingService landed; the generated set is 46 accessors. - The Configuration Options table lists maxRetries and baseRetryDelay, but BasecampClientBuilder exposes no setter for either and the client constructor is internal -- mark them as fixed defaults. - The consoleHooks sample output shows Projects.List; the emitted operation name is Projects.ListProjects (OperationInfo in the generated service). - The same phantom todos.get(projectId, todoId) call appears in the BasecampException and BasecampClient doc comments; fix those too.
The Error Handling fence calls todos.get(projectId: 123, todoId: 456);
the generated signature is get(todoId: Int). The same fence sleeps with
Task.sleep(for: .seconds(...)), which is only available on macOS 13+
while Package.swift declares .macOS(.v12) -- under a macOS 12 target
swiftc rejects it ("'sleep(for:tolerance:clock:)' is only available in
macOS 13.0 or newer"). Use the nanoseconds form the SDK's own retry
path uses.
Also:
- "45 services" is stale since EverythingService landed; the generated
set is 46 accessors.
- The phantom projectId parameter also appears in the BasecampError and
AccountClient doc comments, and Pagination's doc example calls
todos.list(projectId:todolistId:) where the real signature takes
todolistId only; fix those too.
- BasecampError's doc sample carries the same macOS 13-only sleep as
the README fence it mirrors; fix it the same way.
Stale since EverythingService landed; the generated set is 46 service modules.
…site
"List methods return a single page of results by default" inverts
reality: generated list() follows Link: rel="next" up to a 10,000-page
safety cap and returns ListResult<T> (an Array<T> subclass) with
.meta.totalCount / .meta.truncated, bounded per call by maxItems.
Proven behaviorally: against a stubbed fetch serving two pages, list()
made 2 HTTP calls and returned all 3 items; list({maxItems: 2})
returned 2 with truncated: true.
Rewrite the Pagination section around the real behavior and reframe
fetchAllPages/paginateAll as the escape hatch for raw client.GET calls.
The helper callbacks read (r) => r.json() as Promise<any[]> because
bare r.json() is Promise<unknown> under the repo's strict/NodeNext
config and fails tsc --noEmit; all three replacement blocks typecheck
clean.
jeremy
force-pushed
the
docs/readme-example-fixes
branch
from
July 31, 2026 07:01
665442f to
60be468
Compare
jeremy
added a commit
that referenced
this pull request
Jul 31, 2026
) * Ruby README: document the retry contract the transport actually implements The Retry Behavior section was wrong twice over (#510): it listed the retryable statuses as "429, 502, 503, 504" when the transport keys off the error's retryable? classification -- 500 and every other 5xx retry on GET, as do connection-level network errors -- and it described max_retries as "max retry attempts" when it is the TOTAL attempt budget: 3 means one initial attempt plus two retries, and max_retries: 0 sends zero requests and raises "Request failed after 0 attempts". Rewrite the section from the transport's ground truth: GET-only scope with the raw-path carve-outs; classification-based statuses including the read-timeout exception (Faraday maps read timeouts to a status-less non-retryable ApiError, not NetworkError -- only connect-phase timeouts retry); total-attempts semantics; uncapped exponential backoff; Retry-After only via 429; the one-shot 401 refresh replay outside the budget; per-operation retry metadata being inert; and retryable? as the actual transport predicate rather than a hint. Align the config comment, options table, and env-var table (adding the missing BASECAMP_TIMEOUT / BASECAMP_MAX_RETRIES rows), and the same misstatement in config.rb's doc comments. Fixes #510. * TS: make the pagination helper docstring examples pass strict typecheck The fetchAllPages/paginateAll @example blocks use bare (r) => r.json(), which is Promise<unknown> under Node fetch types and fails against the parse callback's Promise<T[]> in any strict/NodeNext project configured like this repo -- the same defect #507 fixed in the README. Mirror the README's proven form: (r) => r.json() as Promise<any[]>. Fixes #511.
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
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.
Six commits, one per SDK. Every defect below was proven against the real SDK before editing, and every corrected snippet was re-proven green. Nothing here touches behavior — READMEs plus five doc comments in Kotlin/Swift source that showed the same phantom signatures.
Ruby —
todos.createtakestodolist_id:, notproject_id:The Quick Start's documented call fails at argument binding, before any HTTP:
The corrected call (with
http_poststubbed on the service singleton) POSTs/todolists/67890/todos.json.Go — five fences didn't compile
Each reproduced offline as the exact compiler error (scratch module +
replacedirective,GOPROXY=off); all corrected snippetsgo build+go vetclean:Also "Requires Go 1.25 or later" → 1.26, matching
go.mod. The README's fragment conventions (undeclaredctx,use(), top-levelreturn err) are established style and stay.Kotlin — error-handling sample didn't compile; tables lied
todos.get(projectId = 123, todoId = 456)— the generated signature isget(todoId: Long)(generated/services/todos.kt).whenover sealedBasecampExceptioncovered 9 of 11 subclasses; a non-exhaustivewhenstatement over a sealed subject is a compile error, and the SDK's ownErrorTest.ktpins all 11 branches ("This ensures all branches compile (exhaustive when)"). AddedDiscoverySelection/DeviceFlowbranches and their Error Types rows.maxRetries/baseRetryDelay, butBasecampClientBuilderhas no setter for either and the client constructor isinternal— marked as fixed defaults.consoleHookssample output showedProjects.List; the generatedOperationInfoemitsProjects.ListProjects.todos.getinBasecampException/BasecampClientdoc comments.Swift — phantom arity plus an API that needs macOS 13
todos.get(projectId: 123, todoId: 456)— the generated signature isget(todoId: Int).Task.sleep(for: .seconds(...))whilePackage.swiftdeclares.macOS(.v12). Proven under a macOS 12 target:Replaced with the nanoseconds form the SDK's own retry path uses (green under the same target). Also fixed the phantom signatures in
BasecampError/AccountClient/Paginationdoc comments.TypeScript —
list()auto-paginates; the docs claimed the opposite"List methods return a single page of results by default" inverts reality. Behavioral proof against a stubbed
fetchserving two pages:The Pagination section now leads with auto-pagination and
ListResult/.meta, showsmaxItems, and reframesfetchAllPages/paginateAllas the escape hatch for rawclient.GETcalls. The helper callbacks read(r) => r.json() as Promise<any[]>because barer.json()isPromise<unknown>under the repo's strict/NodeNext config and failstsc --noEmit; all three replacement blocks typecheck clean under that config.All six SDKs — "45 services" → 46
Stale since
EverythingServicelanded. Verified per SDK: 46 Kotlin accessors, 46 Swift accessors, 46 Ruby service classes (47 files minusbase_service.rb), 46 Python service modules (excluding_base/_async_base), matching SPEC.md's counts.Deliberately not touched
python/README.mdRetry Behavior — current Python README: state the retry contract the code actually implements #502 text.ruby/README.mdRetry Behavior — defective but deserves the Python README: state the retry contract the code actually implements #502-style whole-section treatment; issue to follow.Summary by cubic
Fix incorrect README code samples and doc comments across all SDKs to match real API signatures, pagination behavior, and compile-time constraints. No runtime changes; updates service count references to 46.
todos.createusestodolist_id, notproject_id.result.Projects, passnilto Campfires/Webhooks list calls, fix OTel hooks reassignment (construct client after), addprometheusimport; "Requires Go 1.26".todos.get(todoId), makeBasecampExceptionsample exhaustive withDiscoverySelection/DeviceFlow; markmaxRetries/baseRetryDelayas fixed defaults; console hooks showProjects.ListProjects; fix doc comments.todos.get(todoId), replaceTask.sleep(for:)withnanosecondsfor macOS 12; fix doc comments and the pagination example.list()auto-paginates and returns aListResultwith.meta.totalCountand.truncated; showmaxItems; update raw pagination examples usingfetchAllPages/paginateAllfrom@37signals/basecampwith typedr.json().EverythingService.Written for commit 60be468. Summary will update on new commits.