Skip to content

Conversation

@quinntaylormitchell
Copy link
Collaborator

@quinntaylormitchell quinntaylormitchell commented Nov 3, 2025

Description

This PR adds an annotated image of the new Presto UI, and adds text to explain the annotations.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Built and served docs.

Summary by CodeRabbit

  • Documentation

    • Added a detailed Presto UI guide with a labeled screenshot figure, step‑by‑step descriptions of UI elements (filters, time range, freeform selector, results timeline, results table, query inspector) and usage notes about single‑query limits and query formatting.
    • The same illustrative figure appears in two places for context.
  • Chores

    • Included updated image assets used by the documentation.

@quinntaylormitchell quinntaylormitchell requested a review from a team as a code owner November 3, 2025 16:15
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Adds Git LFS tracking entries for two PNG assets and inserts duplicated Figure 1 blocks into the Presto UI user guide describing UI components and usage notes.

Changes

Cohort / File(s) Summary
Git LFS Configuration
​.gitattributes
Registers clp-presto-ui.png (root) and docs/src/user-docs/clp-presto-ui.png for Git LFS with filter=lfs, diff=lfs, merge=lfs, and -text.
Documentation Update
docs/src/user-docs/guides-using-presto.md
Inserts two identical Figure 1 blocks (duplicated) showing clp-presto-ui.png and a numbered list describing Presto UI elements (filter inputs, time range selector, Freeform selector, results timeline, results table, query inspector) plus usage notes (single query at a time, no trailing semicolon).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Pay attention to the duplicated Figure 1 insertion in docs/src/user-docs/guides-using-presto.md (intentional duplication vs. accidental).
  • Verify image paths in .gitattributes match repository layout and that the LFS rules apply as intended.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly describes the main change: adding an image of the new Presto UI to documentation. The title accurately reflects the primary objective of the changeset.
✨ Finishing touches
🧪 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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed3e590 and b6cfedf.

⛔ Files ignored due to path filters (1)
  • docs/src/user-docs/clp-presto-ui.png is excluded by !**/*.png
📒 Files selected for processing (1)
  • docs/src/user-docs/guides-using-presto.md (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T16:27:50.549Z
Learnt from: davemarco
Repo: y-scope/clp PR: 1198
File: components/webui/server/src/plugins/app/Presto.ts:38-43
Timestamp: 2025-08-25T16:27:50.549Z
Learning: In the CLP webui Presto configuration, host and port are set via package settings (configurable), while user, catalog, and schema are set via environment variables (environment-specific). This mixed approach is intentional - settings are typically set by package and some values don't need to be package-configurable.

Applied to files:

  • docs/src/user-docs/guides-using-presto.md
🪛 markdownlint-cli2 (0.18.1)
docs/src/user-docs/guides-using-presto.md

201-201: Link fragments should be valid

(MD051, link-fragments)


212-212: Link fragments should be valid

(MD051, link-fragments)

⏰ 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). (2)
  • GitHub Check: package-image
  • GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (2)
docs/src/user-docs/guides-using-presto.md (2)

206-206: Specify the image directory path.

Line 206 references clp-presto-ui.png without a directory prefix. Verify the image location and update the path accordingly (e.g., ./clp-presto-ui.png if in the same directory, or a relative path to the assets folder).


201-240: No duplication found. The review comment's concern is unfounded.

The verification confirms:

  • The (figure-1)= reference definition appears only once (line 203)
  • Two links to Figure 1 exist at lines 201 and 212, both pointing to the same single definition
  • The image file clp-presto-ui.png exists at the correct location

Having multiple references to a single figure definition is standard practice in documentation and is correct. The AI-generated summary mentioned in the original review comment mischaracterized the code structure.

Likely an incorrect or invalid review comment.

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