Skip to content

Address remaining README review feedback - #2

Merged
elijahpetty merged 1 commit into
deephaven-examples:add-examplefrom
margaretkennedy:readme-review-fixes
Sep 28, 2026
Merged

elijahpetty merged 1 commit into
deephaven-examples:add-examplefrom
margaretkennedy:readme-review-fixes

Conversation

@margaretkennedy

Copy link
Copy Markdown

Follow-up to #1, targeting the add-example branch. Addresses the remaining README-related review feedback from @chipkent (does not touch setuptools-deployment.md, which is moving to the core docs PR).

  • Server snippets: fixes the duplicated server = server = Server(...) assignment in the root README (two places). Keeps the two-line server = Server(...) / server.start() form rather than server = Server(...).start(), because Server.start() returns None — the two-line form already holds a reference, so the server isn't garbage collected.
  • Consistent entry-point naming: renames the my_dh_cli command function app → main to match my_dh_toolkit (cli.py, __main__.py, pyproject.toml, READMEs, and the "Adapt an example" snippet).
  • Deferred imports: my_dh_cli/cli.py now has the same comment as the toolkit explaining why deephaven is imported inside the function; Example 2's "What to study" notes it too.
  • Readability: splits the long __init__.py explanation in Example 3 into short bullets.
  • Redundancy: removes the toolkit summary under the pattern table, which duplicated the Example 3 intro.
  • Duplicated library code: Example 3 now states that queries.py/utils.py are deliberate copies of my_dh_library so each example stays self-contained.

Verified the renamed my_dh_cli.cli:main entry point resolves and python -m my_dh_cli --help runs; not run end-to-end against a live Deephaven server.

🤖 Generated with Claude Code

- Fix duplicated `server = server =` assignment in root README snippets
- Rename my_dh_cli entry function `app` to `main` to match my_dh_toolkit
- Explain deferred deephaven imports in my_dh_cli (code comment + README)
- Split the long my_dh_toolkit __init__.py explanation into short bullets
- Remove redundant toolkit summary under the pattern table
- Note that toolkit's queries.py/utils.py are deliberate copies

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@elijahpetty
elijahpetty merged commit acda4a6 into deephaven-examples:add-example Sep 28, 2026
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.

2 participants