docs(native): Add documentation for Presto C++ Installation#26718
docs(native): Add documentation for Presto C++ Installation#26718steveburnett merged 2 commits intoprestodb:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds new Sphinx documentation describing how to install and run the Presto C++ worker, including build, configuration, and integration steps, by introducing a dedicated installation guide and wiring it into the existing Presto C++ docs hierarchy. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Since you’ve moved content under
presto_cpp/installation.rst, make surepresto-cpp.rst(and any relevant toctrees) links to this new doc so it’s discoverable from the main navigation. - Check for any existing references or deep links to
presto-cpp.rstthat should now point to the newpresto_cpp/installation.rstpath to avoid broken links in the published docs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since you’ve moved content under `presto_cpp/installation.rst`, make sure `presto-cpp.rst` (and any relevant toctrees) links to this new doc so it’s discoverable from the main navigation.
- Check for any existing references or deep links to `presto-cpp.rst` that should now point to the new `presto_cpp/installation.rst` path to avoid broken links in the published docs.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
steveburnett
left a comment
There was a problem hiding this comment.
Thanks for the doc! I made a lot of notes, mostly formatting and phrasing to fit the tone of technical documentation. Some suggestions and ideas for you to consider.
4a14b2d to
e3a5a8d
Compare
steveburnett
left a comment
There was a problem hiding this comment.
Thanks for the revision! This is looking better, so this review is mostly focused on revision suggestions to improve the consistency of the tone with the rest of the Presto documentation.
I have tried to explain my comments and suggestions. Let me know if you have any questions or concerns about these and we can discuss them if you want to.
c5d6a0a to
e3a5a8d
Compare
|
Hi, @saurabhmahawar! You have comments showing screenshots of changes, but those changes are not in the file in this PR. Have you pushed your changes to GitHub? When you have time, please take a look at the rest of my comments from my previous review. Thanks! |
e3a5a8d to
93b48e3
Compare
steveburnett
left a comment
There was a problem hiding this comment.
Thanks for the update, good progress! A few more suggestions.
| node.location=docker | ||
| node.id=worker-1 | ||
|
|
||
| * ``node.internal-address=worker-1``: This setting matches the service name defined in Docker Compose. |
There was a problem hiding this comment.
Suggest replacing "Docker Compose" with a link to the heading "Create docker-compose.yml" in line 178.
steveburnett
left a comment
There was a problem hiding this comment.
LGTM! (docs)
Pull updated branch, new local doc build, looks good. Thanks!
Description
Added new documentation explaining how to use the Presto C++ engine.
The documentation provides step-by-step instructions for configuring, and running the Presto C++ worker
Motivation and Context
There was no consolidated or beginner-friendly documentation for Presto C++ in the open-source project.
Users often had difficulty understanding how to build and run the C++ worker, what dependencies were required, and how it integrates with a Presto coordinator.
Impact
There is no performance impact.
Test Plan
Contributor checklist
Release Notes