Skip to content

docs: add JSDoc-first commenting guidelines and test file location rules#10

Merged
betegon merged 1 commit intomainfrom
docs/jsdoc-commenting-guidelines
Jan 15, 2026
Merged

docs: add JSDoc-first commenting guidelines and test file location rules#10
betegon merged 1 commit intomainfrom
docs/jsdoc-commenting-guidelines

Conversation

@betegon
Copy link
Member

@betegon betegon commented Jan 15, 2026

Summary

  • Add Commenting & Documentation (JSDoc-first) section to AGENTS.md with clear rules for when to use JSDoc vs inline comments
  • Update Testing section to explicitly state test files must go in packages/cli/test/
  • Add test directory to File Locations table

Changes

JSDoc Guidelines

  • Prefer JSDoc over inline comments
  • Required on all exported functions, classes, types, and interfaces
  • Document intent, behavior, constraints, edge cases, and side effects
  • Inline comments only for "why" explanations, not "what" narration

Test Location Rules

  • Test files go in packages/cli/test/, not alongside source files
  • Mirror source structure (e.g., test/lib/config.test.ts for src/lib/config.ts)
  • E2E tests in test/e2e/

@betegon betegon merged commit df590a5 into main Jan 15, 2026
@betegon betegon deleted the docs/jsdoc-commenting-guidelines branch January 15, 2026 16:56
BYK added a commit that referenced this pull request Mar 4, 2026
BugBot #9: Wrap CachePolicy.fromObject() and related calls in try-catch
inside getCachedResponse(). A corrupted or version-incompatible policy
object now triggers a cache miss (and best-effort cleanup of the broken
entry) instead of crashing the API request.

BugBot #10: Add missing `await` to clearAuth() calls in project/list
tests at lines 1080 and 1362 to prevent floating promises.
BYK added a commit that referenced this pull request Mar 4, 2026
…parallelize cleanup

Review Round 3 — 15 human review comments addressed:

#1: login.ts — Replace await .catch() with proper try/await/catch blocks
#2: whoami.ts + all commands — Export FRESH_ALIASES constant from list-command.ts
    to reduce boilerplate; update 15 command files to use it
#3: response-cache.ts — Bump immutable TTL from 1hr to 24hr (events/traces
    never change once created)
#4–6: response-cache.ts — Restructure URL_TIER_PATTERNS as Record<TtlTier, RegExp[]>,
    combine duplicate regex patterns into single alternations
#7: response-cache.ts — Replace localeCompare with simple < comparison for
    ASCII URL query param sorting
#8: response-cache.ts — Remove try-catch in normalizeUrl (URLs reaching the
    cache already came from a fetch, always valid)
#9: response-cache.ts — Link immutableMinTimeToLive to FALLBACK_TTL_MS.immutable
    instead of hardcoded magic number
#10: response-cache.ts — Use Object.fromEntries(headers.entries()) instead of
    manual forEach loop
#11: response-cache.ts — Remove unnecessary await on fire-and-forget unlink in
    catch block
#12: response-cache.ts — Add expiresAt field to CacheEntry for O(1) expiry
    checks during cleanup (no CachePolicy deserialization needed)
#13–15: response-cache.ts — Parallelize cache I/O (collectEntryMetadata,
    deleteExpiredEntries, evictExcessEntries) using p-limit-style concurrency
    limiter (max 8 concurrent)
BYK added a commit that referenced this pull request Mar 4, 2026
BugBot #9: Wrap CachePolicy.fromObject() and related calls in try-catch
inside getCachedResponse(). A corrupted or version-incompatible policy
object now triggers a cache miss (and best-effort cleanup of the broken
entry) instead of crashing the API request.

BugBot #10: Add missing `await` to clearAuth() calls in project/list
tests at lines 1080 and 1362 to prevent floating promises.
BYK added a commit that referenced this pull request Mar 4, 2026
BugBot #9: Wrap CachePolicy.fromObject() and related calls in try-catch
inside getCachedResponse(). A corrupted or version-incompatible policy
object now triggers a cache miss (and best-effort cleanup of the broken
entry) instead of crashing the API request.

BugBot #10: Add missing `await` to clearAuth() calls in project/list
tests at lines 1080 and 1362 to prevent floating promises.
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