Skip to content

LCORE-1077: better doc in scripts#879

Merged
tisnik merged 3 commits intolightspeed-core:mainfrom
tisnik:lcore-1077-better-doc-in-scripts
Dec 8, 2025
Merged

LCORE-1077: better doc in scripts#879
tisnik merged 3 commits intolightspeed-core:mainfrom
tisnik:lcore-1077-better-doc-in-scripts

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Dec 8, 2025

Description

LCORE-1077: better doc in scripts

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: CodeRabbitAI
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-1077

Summary by CodeRabbit

  • Documentation
    • Enhanced internal documentation with expanded and clarified descriptions across utility scripts for improved maintainability.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 8, 2025

Walkthrough

Docstrings expanded for three utility scripts: gen_doc.py (generate_docfile, generate_documentation_on_path functions), generate_openapi_schema.py (read_version_from_openapi, read_version_from_pyproject functions), and query_llm.py (main function). No functional code or control flow changes.

Changes

Cohort / File(s) Change Summary
Docstring expansions
scripts/gen_doc.py, scripts/generate_openapi_schema.py, scripts/query_llm.py
Expanded docstrings for utility functions describing behavior, parameters, return values, and exit codes; no code or signature modifications

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly summarizes the main change: expanding docstrings in script files for better documentation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
scripts/generate_openapi_schema.py (2)

26-32: Docstring for read_version_from_openapi matches behavior; consider a brief Args section

The description and return docs are accurate. For closer alignment with Google-style docstrings, you might add an Args: section documenting filename as the path to the OpenAPI JSON file, but this is optional.

As per coding guidelines, brief Google-style docstrings are preferred.


43-49: read_version_from_pyproject docstring is accurate; optional polish on structure

The docstring clearly states that PDM is used (including dynamic versions) and documents the return value. If you want to standardize on Google style, you could structure it as a short summary plus a Returns: block (already present) and optionally an Args: block (even if empty here) for consistency with neighboring functions.

As per coding guidelines, this already meets the brief/clear-docstring requirement.

scripts/gen_doc.py (2)

13-22: generate_docfile docstring is clear; optional wording/style tweaks

The docstring correctly describes how README.md is produced (header, per-file link, first docstring line, then filename). If you want to align more closely with Google-style docstrings, you could:

  • Add an Args: section documenting directory (used only for the header text).
  • Slightly tighten wording, e.g., “For each .py file in the current working directory, the function writes…”.
    These are purely cosmetic.

As per coding guidelines, the function now has a clear, brief description; style tweaks are optional.


46-53: Clarify scope and grammar in generate_documentation_on_path docstring

The docstring matches the behavior (generate README.md for Python sources in the given directory), but you might:

  • Fix minor grammar: “Generate documentation for all the sources found in the path.” / “This function generates a README.md…”.
  • Make the scope explicit, e.g., “Generate a README.md for Python source files directly in the given directory.” (since recursion happens in main, not inside this function).
  • Optionally switch Parameters: to Args: to better match Google-style conventions.

As per coding guidelines, this already provides a useful brief description; these are polish suggestions only.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c10b1eb and 98d398f.

📒 Files selected for processing (3)
  • scripts/gen_doc.py (2 hunks)
  • scripts/generate_openapi_schema.py (2 hunks)
  • scripts/query_llm.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:58:04.410Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to src/**/*.py : All functions must have docstrings with brief descriptions following Google Python docstring conventions

Applied to files:

  • scripts/gen_doc.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: library mode / ci
  • GitHub Check: E2E: server mode / ci
  • GitHub Check: E2E: server mode / azure
  • GitHub Check: E2E: library mode / azure
🔇 Additional comments (1)
scripts/query_llm.py (1)

15-30: Expanded main() docstring accurately reflects CLI behavior and exit codes

The docstring lines up with the actual argument parsing, request/response handling, stderr diagnostics, and the four exit codes (0–3). This is a solid, self-contained description for users invoking the script.

@tisnik tisnik merged commit f20efb9 into lightspeed-core:main Dec 8, 2025
21 of 25 checks passed
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