-
Notifications
You must be signed in to change notification settings - Fork 0
Christian/live view docs update #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What Changed
This PR updates the documentation for our Live View feature in observability/live-view.mdx
. The changes aim to improve clarity and provide more detailed configuration instructions. Key updates include renaming the 'Query parameters' section to 'Live view configuration', adding a full example URL, and clarifying framerate options.
Risks / Concerns
The review identified a small but important error in the example URL provided in observability/live-view.mdx
. The URL was missing an ampersand (&
) between query parameters, which would cause it to fail if copied by a user. Please apply the suggestion to ensure the documentation is accurate.
1 files reviewed | 1 comments | Review on Mesa | Edit Reviewer Settings
observability/live-view.mdx
Outdated
Configuring the live view by setting query parameters on your `browser_live_view_url`. | ||
|
||
``` | ||
https://api.onkernel.com/browser/live?w=1024&h=768&r=60readOnly=false&jwt= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example URL is missing an ampersand (&) between the r=60
and readOnly=false
parameters. It should be:
https://api.onkernel.com/browser/live?w=1024&h=768&r=60readOnly=false&jwt= | |
https://api.onkernel.com/browser/live?w=1024&h=768&r=60&readOnly=false&jwt= |
Type: Logic | Severity: Medium
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
observability/live-view.mdx
Outdated
Configuring the live view by setting query parameters on your `browser_live_view_url`. | ||
|
||
``` | ||
https://api.onkernel.com/browser/live?w=1024&h=768&r=60readOnly=false&jwt= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Description
Please provide an explanation of the changes you've made:
[Describe what this PR does and why]
Implementation Checklist
Testing
mintlify dev
works (see installation here)Docs
Visual Proof
Please provide a screenshot or video demonstrating that your changes work locally:
[Drag and drop your screenshot/video here or use the following format:]
]
[
Related Issue
Fixes [Github issue link]
[If this corresponds to a fix from another Kernel OSS repo, include this:]
Fixes [Link to other repo]
[Replace with actual issue link, e.g., Fixes https://github.com/username/repo/issues/123]
Additional Notes
[Any additional context, concerns, or notes for reviewers]
TL;DR
Improved the clarity and detail of the Live View documentation, particularly regarding configuration and framerate settings.
Why we made these changes
To enhance user understanding of Live View configuration options and provide more precise guidance on supported framerate ranges.
What changed?
Validation
mintlify dev
works locally.