Skip to content

Add docs.nats.io examples to main with typecheck CI - #425

Open
Jarema wants to merge 2 commits into
mainfrom
docs-examples-to-main
Open

Add docs.nats.io examples to main with typecheck CI#425
Jarema wants to merge 2 commits into
mainfrom
docs-examples-to-main

Conversation

@Jarema

@Jarema Jarema commented Jul 21, 2026

Copy link
Copy Markdown
Member

Moves the documentation examples from the docs-io-nats-examples, core-docs and jetstream-docs branches into docs-io-nats-examples/ on main (72 standalone TS scripts, additive union).

Unlike the other clients, merging alone gives these no CI coverage — the dir is outside every module matrix, not a workspace member, and deno only typechecks entrypoints it's told about. So this PR also adds a small docs-examples job to test.yml: deno check docs-io-nats-examples/*.ts (root deno.json scopes resolve the @nats-io/* imports to the local modules).

Verified locally with deno 2.7.1: all 72 files typecheck clean.

Why: the new docs.nats.io build fetches these snippets at build time; on main with a check job they can't rot silently or vanish with a pruned branch. Please keep the docs branches until the docs repo's fetch config is switched to main (follow-up PR there).

🤖 Generated with Claude Code

Consolidates the docs-io-nats-examples, core-docs and jetstream-docs
branches' examples into docs-io-nats-examples/ on main. The dir is
outside every module matrix and imported by nothing, so deno never
typechecks it; adds a docs-examples job to test.yml running
'deno check docs-io-nats-examples/*.ts'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Jarema's task in 4m 26s —— View job


PR Review

  • Gather context and review PR changes
  • Analyze CI workflow changes
  • Review example TypeScript files for correctness, async patterns, and resource cleanup
  • Check for cross-runtime compatibility issues
  • Post inline comments on specific issues
  • Post overall review summary

Summary

The PR is structurally sound — 72 standalone typecheck-only examples moved from pruned branches into a permanent, CI-guarded location is the right call. The async/resource patterns across the JetStream and core examples are generally correct (proper await nc.drain(), Promise.allSettled for async batch publish, ackAck where needed). Two fixable issues and one process question:

Floating Promises (2 files)

basics-subscribe.ts:22 and getting-started-subscribe.ts:22 (identical content):

delay(1000).then(() => {
  nc.publish("weather.updates", "Weather: Sunny in NYC");
})  // no .catch()

nc.publish is synchronous so the practical risk is very low, but as documentation this teaches an anti-pattern. Add .catch(console.error) to make the intent explicit and avoid training readers to write fire-and-forget Promise chains. Inline suggestion posted.

Missing Apache 2.0 license headers (10 files)

CLAUDE.md requires the license header on every source file. These 10 files are missing it:

  • learn-jetstream-pull-consumers-consumeContinuous.ts
  • learn-jetstream-pull-consumers-fetchBatch.ts
  • learn-jetstream-pull-consumers-emptyFetch.ts
  • learn-jetstream-ordered-consumer-read.ts
  • learn-jetstream-worker-pool-worker.ts
  • learn-jetstream-worker-pool-max-pending.ts
  • learn-jetstream-worker-pool-redelivery-count.ts
  • learn-jetstream-shaping-the-stream-discardNew.ts
  • learn-jetstream-shaping-the-stream-perSubjectLimit.ts
  • learn-jetstream-shaping-the-stream-setLimits.ts

CI job not wired into branch protection

The docs-examples job runs but is not in the coveralls-finish needs list (which is fine — it produces no coverage). However, if the intent is to block merges when examples fail to typecheck, docs-examples must be added to the repo's required status checks in branch protection settings. The workflow job alone won't enforce that.


delay(1000).then(() => {
nc.publish("weather.updates", "Weather: Sunny in NYC");
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Floating Promise — the .then() fires without a .catch(). If delay() rejects for any reason the error is silently swallowed. As documentation this teaches the wrong pattern. Either await the whole thing (wrapping in an immediately-invoked async block) or add .catch(console.error):

Suggested change
})
delay(1000).then(() => {
nc.publish("weather.updates", "Weather: Sunny in NYC");
}).catch(console.error);

Same issue exists verbatim in getting-started-subscribe.ts.

@@ -0,0 +1,23 @@
// import the connect function from a transport

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing the Apache 2.0 license header. CLAUDE.md states: "All source files must include the Apache 2.0 license header." Ten files in this PR are missing it:

  • learn-jetstream-pull-consumers-consumeContinuous.ts
  • learn-jetstream-pull-consumers-fetchBatch.ts
  • learn-jetstream-pull-consumers-emptyFetch.ts
  • learn-jetstream-ordered-consumer-read.ts
  • learn-jetstream-worker-pool-worker.ts
  • learn-jetstream-worker-pool-max-pending.ts
  • learn-jetstream-worker-pool-redelivery-count.ts
  • learn-jetstream-shaping-the-stream-discardNew.ts
  • learn-jetstream-shaping-the-stream-perSubjectLimit.ts
  • learn-jetstream-shaping-the-stream-setLimits.ts

uses: denoland/setup-deno@v2
with:
deno-version: 2.7.x
- name: Typecheck docs examples

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The docs-examples job is not listed in coveralls-finish's needs. That's fine for coverage (docs examples produce no coverage data), but it means coveralls-finish can pass even when docs-examples fails. If this job should truly gate merges, it needs to be added to the required status checks in the repo's branch protection settings — the workflow alone won't enforce it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Jarema added a commit to nats-io/nats.docs.v2 that referenced this pull request Jul 27, 2026
Every client's docs.nats.io examples are now merged into their repo's
main branch, so point EXAMPLES_CONFIG there and stop tracking the
per-chapter staging branches (doc-examples, core-docs, jetstream-docs).

Covers nats.go, nats.rs, nats.py, nats.java, nats.net and the matching
Orbit repos. Verified every configured path resolves on main before
switching.

nats.js stays on its branches: nats-io/nats.js#425 is still open and
main has no docs-io-nats-examples directory yet.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UbUdMQBRUZp8MfocV15vQ5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant