-
Notifications
You must be signed in to change notification settings - Fork 16
docs: Add deployment instructions (resolves #217). #236
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
WalkthroughA new documentation file was introduced to provide detailed guidance on deploying the log viewer's built distribution, including hosting, security, and performance optimizations. The developer guide index was updated to include and link to this new deployment documentation, integrating it into the build-related navigation structure. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Documentation
participant StaticHost
participant WebServer
User ->> Documentation: Reads deployment guide
Documentation -->> User: Provides hosting, security, and optimization steps
User ->> StaticHost: Deploys built 'dist' directory
StaticHost -->> User: Hosts static files
User ->> WebServer: Configures compression and MIME types (if applicable)
WebServer -->> User: Serves optimized assets
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/src/dev-guide/building-getting-started.md (5)
39-40
: Section Header: Deploying the Distribution
The new "Deploying the distribution" header clearly introduces a dedicated section for deployment guidance. Consider adding a brief introductory sentence explaining the benefits of static file hosting to further contextualise the instructions.
43-56
: Static File Hosting Guidance
This section effectively outlines various static hosting options, including GitHub Pages, Netlify, Vercel, and others. The bullet list and use of a backslash for line breaks (e.g. on line 47) help maintain readability. Please review that the backslash behavior renders consistently across all Markdown viewers.🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Possible missing comma found.
Context: ... the dist/ folder to any static hosting service such as: * **[GitHub Pages][github-pag...(AI_HYDRA_LEO_MISSING_COMMA)
64-77
: Warning Admonition and Link Consistency
The warning block clearly communicates the potential issue with duplicate HTTP requests due to incorrect MIME type configuration. However, note that the inline link to the Emscripten issue is directly embedded, while a reference definition for[emscripten-issue-18468]
is provided later. For consistency, consider converting the inline link to use the reference or removing the unused reference.
104-109
: Unused Reference Link
The reference for[emscripten-issue-18468]
appears to be unused since the warning uses an inline link. To avoid confusion and satisfy markdown linting (MD053), either remove this reference or update the warning to use the reference style.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
104-104: Link and image reference definitions should be needed
Unused link or image reference definition: "emscripten-issue-18468"(MD053, link-image-reference-definitions)
45-47
: Comma Usage in List Formatting
A static analysis hint flagged a possible missing comma in the list of hosting services. Although the current formatting is clear, consider reviewing the punctuation to ensure it meets both grammatical standards and any tooling expectations.🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Possible missing comma found.
Context: ... the dist/ folder to any static hosting service such as: * **[GitHub Pages][github-pag...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/building-getting-started.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/dev-guide/building-getting-started.md
[uncategorized] ~45-~45: Possible missing comma found.
Context: ... the dist/ folder to any static hosting service such as: * **[GitHub Pages][github-pag...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/building-getting-started.md
104-104: Link and image reference definitions should be needed
Unused link or image reference definition: "emscripten-issue-18468"
(MD053, link-image-reference-definitions)
🔇 Additional comments (4)
docs/src/dev-guide/building-getting-started.md (4)
41-41
: Deployment Overview Clarity
The directive "To deploy the application, serve the contents of thedist
directory using a static file host." is concise and immediately clear.
59-63
: MIME Types Section Clarity
The instructions to serve WebAssembly (.wasm
) files with the MIME typeapplication/wasm
are succinct and provide necessary technical detail.
79-88
: Compression Guidelines
The Compression section effectively advises on enabling Gzip, Brotli, or Zstandard for various file types and explains how to verify the implementation via response headers. The reference to MDN documentation further supports the guidance.
89-103
: HTTPS / TLS Recommendations
The HTTPS / TLS section thoroughly explains the importance of serving the application over HTTPS, especially to prevent mixed content issues. The alternatives provided for loading external files are practical.
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.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/dev-guide/building-getting-started.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/building-getting-started.md
48-48: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
50-50: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
51-51: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
65-65: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
66-66: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
81-81: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
82-82: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
86-86: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
92-92: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
93-93: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
98-98: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
104-104: Link and image reference definitions should be needed
Unused link or image reference definition: "emscripten-issue-18468"
(MD053, link-image-reference-definitions)
🔇 Additional comments (3)
docs/src/dev-guide/building-getting-started.md (3)
39-42
: New section introduction is well-structured
The “## Deploying the distribution” heading and its introductory line fit seamlessly into the Getting Started guide and clearly frame the deployment steps.
59-63
: MIME type guidance is clear and accurate
The note about serving.wasm
files withapplication/wasm
correctly highlights a common pitfall. The inline reference to the Emscripten issue provides immediate context.
89-96
: HTTPS/TLS guidance is comprehensive
Emphasizing mixed-content restrictions and recommending HTTPS aligns perfectly with best practices. Trim the trailing spaces on lines 92–93 as reported by markdownlint.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
92-92: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
93-93: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
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 document looks good and clearly points out #217.
I don't have many comments, aside from the markdown style suggestions from Coderabbit -- though I’m not sure how much those really matter, since the document already reads well.
:::{warning} | ||
Failing to do so can cause the Emscripten-generated JavaScript wrapper to make | ||
two HTTP requests for the same `.wasm` file, leading to unnecessary network overhead. This issue is | ||
detailed in [Emscripten issue #18468](https://github.com/emscripten-core/emscripten/issues/18468). |
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.
Can we use a link reference here? Or is it that it's in a warning block and link refs aren't supported there?
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.
good catch. we should be able to use link reference here
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.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/src/dev-guide/building-deploying.md
(1 hunks)docs/src/dev-guide/index.md
(2 hunks)
🔇 Additional comments (2)
docs/src/dev-guide/index.md (2)
18-23
: New "Deploying" card is consistent with existing style
The added grid-item-card for Deploying follows the same gerund-based pattern used by the other cards (Building, Contributing). The title, link target, and description are all aligned.
44-46
: Building toctree updated correctly
Includingbuilding-deploying
in the hidden toctree under “Building” ensures the new section is part of the build-related navigation. Confirm thatbuilding-deploying.md
exists at that path and renders as expected in the sidebar.
## Enabling compression | ||
|
||
To improve load times, you can reduce file transfer sizes by enabling compression for the files in | ||
the distribution (`.css`, `.js`, `.html`, and `.wasm` files). You can enable compression by | ||
configuring the static file host to support popular [content-encoding][mdn-content-encoding] methods | ||
(e.g., `gzip`). | ||
|
||
You can verify that compression is working by inspecting the response headers for any of the static | ||
assets. The `Content-Encoding` header should show a supported content encoding method (e.g., `br`, | ||
`gzip`, or `zstd`). |
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.
🧹 Nitpick (assertive)
Include a quick command to verify compression
Readers will benefit from a concrete example to confirm gzip/ Brotli is enabled. For example:
Add to “Enabling compression” section:
You can run:
```bash
curl -I https://<your-host>/main.js | grep -i 'Content-Encoding'
to verify that gzip
, br
, or another encoding is applied.
<details>
<summary>🤖 Prompt for AI Agents</summary>
In docs/src/dev-guide/building-deploying.md around lines 48 to 57, add a
concrete example command to verify compression is enabled. Include a bash
snippet using curl with the -I option to fetch headers from a static asset URL
and pipe it to grep to filter for 'Content-Encoding'. This will help readers
quickly confirm if gzip, br, or another encoding is applied by checking the
response headers.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
## Configuring a WebAssembly MIME type | ||
|
||
To avoid unnecessary downloads of WebAssembly (`.wasm`) files (see | ||
[emscripten-core/emscripten#18468]), the server should be configured to serve such files using the | ||
`application/wasm` MIME type. The following web server deployments use this MIME type by default: | ||
|
||
* The [Apache HTTP Server][apache-httpd] on Debian-based systems (e.g., Debian v10+ or Ubuntu | ||
v20.04+) | ||
* This deployment relies on the system's `/etc/mime.types` file which is included in the | ||
media-types v3.62 package). | ||
* [Nginx] v1.21.0+. | ||
|
||
If your web server is not one of the above, ensure it is configured to use the aforementioned MIME | ||
type. | ||
|
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.
🧹 Nitpick (assertive)
Provide sample server configuration for WebAssembly MIME type
Listing supported servers is great, but a short snippet would help users configure others. For example, for Nginx:
location ~ \.wasm$ {
add_header Content-Type application/wasm;
}
🤖 Prompt for AI Agents
In docs/src/dev-guide/building-deploying.md around lines 59 to 73, add a sample
server configuration snippet for setting the WebAssembly MIME type to help users
configure servers not listed. For example, include an Nginx configuration block
showing how to add the Content-Type header for .wasm files. This will provide
practical guidance alongside the existing server list.
### Troubleshooting | ||
|
||
1. If users of the deployed distribution want to load log files from a different origin than the | ||
static file host, you'll need to ensure that the host serving the log files supports | ||
[cross-origin resource sharing (CORS)][mdn-cors]. | ||
2. If your static file host serves the log viewer over a secure connection, modern browsers won't | ||
allow users to load log files over an insecure connection due to | ||
[mixed content restrictions][mdn-mixed-content-restrictions]. |
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.
🧹 Nitpick (assertive)
Enhance troubleshooting guidance
The troubleshooting section identifies CORS and mixed-content issues but doesn’t show how to resolve them. Consider adding brief examples or links for common hosting platforms, e.g.:
- Ensure response header `Access-Control-Allow-Origin: *` on your log-file server.
- For mixed content, configure your embed to use `https://…` or update your server’s TLS certs.
🤖 Prompt for AI Agents
In docs/src/dev-guide/building-deploying.md around lines 39 to 46, the
troubleshooting section mentions CORS and mixed content issues but lacks
practical resolution steps. Enhance this section by adding brief examples or
instructions on how to fix these issues, such as setting the response header
Access-Control-Allow-Origin to * on the log file server for CORS, and advising
to use HTTPS URLs or update TLS certificates to resolve mixed content problems.
Optionally, include links to documentation for common hosting platforms to guide
users.
# Deploying a distribution | ||
|
||
To deploy a [built](building-getting-started.md) distribution of the log viewer, you'll need to do | ||
the following: | ||
|
||
1. [Serve the files using a static file host](#static-file-hosting). | ||
2. Apply any of the following optimizations: | ||
* [Compression](#enabling-compression) | ||
* [Configure a MIME type for WebAssembly files](#configuring-a-webassembly-mime-type) | ||
|
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.
Fix internal link to Getting Started doc
The Markdown link currently points to building-getting-started.md
, which may not resolve under Sphinx/MyST (extensions are dropped). Please remove the .md
extension so it matches the toctree entry:
- To deploy a [built](building-getting-started.md) distribution of the log viewer, you'll need to do
- the following:
+ To deploy a [built](building-getting-started) distribution of the log viewer, you'll need to do
+ the following:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Deploying a distribution | |
To deploy a [built](building-getting-started.md) distribution of the log viewer, you'll need to do | |
the following: | |
1. [Serve the files using a static file host](#static-file-hosting). | |
2. Apply any of the following optimizations: | |
* [Compression](#enabling-compression) | |
* [Configure a MIME type for WebAssembly files](#configuring-a-webassembly-mime-type) | |
# Deploying a distribution | |
To deploy a [built](building-getting-started) distribution of the log viewer, you'll need to do | |
the following: | |
1. [Serve the files using a static file host](#static-file-hosting). | |
2. Apply any of the following optimizations: | |
* [Compression](#enabling-compression) | |
* [Configure a MIME type for WebAssembly files](#configuring-a-webassembly-mime-type) |
🤖 Prompt for AI Agents
In docs/src/dev-guide/building-deploying.md lines 1 to 10, the internal Markdown
link to the Getting Started document incorrectly includes the .md extension,
which may not resolve properly in Sphinx/MyST. Remove the .md extension from the
link target so it matches the toctree entry and resolves correctly.
## Static file hosting | ||
|
||
You can deploy the built distribution (the `dist` directory) to any static hosting service such as: | ||
|
||
* [GitHub Pages][github-pages] | ||
|
||
:::{tip} | ||
If you fork this repository and [enable GitHub Actions][enable-gh-actions] in your fork, every | ||
push will trigger the [deployment workflow][gh-workflow-deploy-gh-pages] to deploy the site to | ||
`https://<your-github-username>.github.io/yscope-log-viewer/` using GitHub Pages. |
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.
🧹 Nitpick (assertive)
Unify link reference style for hosting services
Some entries use [Netlify]
and [Vercel]
, while Cloudflare Pages is written as [Cloudflare Pages][cloudflare-pages]
. For consistency, pick one style. For example, simplify Cloudflare Pages to:
-* [Cloudflare Pages][cloudflare-pages]
+* [Cloudflare Pages]
and rely on the link definition at the bottom.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docs/src/dev-guide/building-deploying.md around lines 11 to 20, the link
references for hosting services are inconsistent, with some using simple
bracketed names like [Netlify] and [Vercel], while Cloudflare Pages uses a
double reference style like [Cloudflare Pages][cloudflare-pages]. To fix this,
unify the style by simplifying Cloudflare Pages to just [Cloudflare Pages] and
ensure all hosting service links follow this single bracket style, relying on
the link definitions at the bottom of the document.
Description
Checklist
breaking change.
Validation performed
Ubuntu 22.04.3 LTS
server, cloned the repo and checked out the branch.docs:serve
and accessed http://127.0.0.1:8080/dev-guide/building-getting-started.htmlSummary by CodeRabbit