Repository navigation
docs: Reduce git LFS bandwidth - #421
Merged
Merged
Conversation
Signed-off-by: Ilya Boyandin <[email protected]>
Signed-off-by: Ilya Boyandin <[email protected]>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated references to media from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/sync-examples.yml (1)
67-68: Using./.instead of/*prevents edge-case failures if the media directory becomes empty.Line 68 uses
*, which remains a literal glob ifsource/docs/public/media/examples/becomes empty, causingcpto fail. Copyingsource/docs/public/media/examples/.instead handles both populated and empty directories safely and preserves nested files. While the directory currently has files, this defensive change guards against future states where media content might not exist.Suggested fix
- if [ -d "source/docs/public/media/examples" ]; then - cp -r source/docs/public/media/examples/* target/media/examples/ + if [ -d "source/docs/public/media/examples" ]; then + cp -r source/docs/public/media/examples/. target/media/examples/ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync-examples.yml around lines 67 - 68, Change the cp invocation that copies examples to use the directory dot notation instead of a glob so it works when the source is empty; replace the occurrence of source/docs/public/media/examples/* in the workflow step with source/docs/public/media/examples/. (This updates the cp command in the sync-examples workflow so cp -r source/docs/public/media/examples/. target/media/examples/ safely handles both populated and empty source directories.)
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/deckgl-mosaic/README.md`:
- Line 11: Replace the invalid alt attribute on the <video> element with
accessible alternatives: add an aria-label (and/or aria-labelledby) on the same
<video> tag to provide a text description for assistive tech, and include
descriptive fallback text between the <video>...</video> tags so non-supporting
browsers and screen readers can access the description; update the existing
<video
src="https://sqlrooms.org/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4" ...>
element accordingly.
---
Nitpick comments:
In @.github/workflows/sync-examples.yml:
- Around line 67-68: Change the cp invocation that copies examples to use the
directory dot notation instead of a glob so it works when the source is empty;
replace the occurrence of source/docs/public/media/examples/* in the workflow
step with source/docs/public/media/examples/. (This updates the cp command in
the sync-examples workflow so cp -r source/docs/public/media/examples/.
target/media/examples/ safely handles both populated and empty source
directories.)
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea270e98-4aa2-4401-b40d-fea8a84cb5fa
⛔ Files ignored due to path filters (18)
docs/public/media/case-studies/cosmograph-2.0-967px.mp4is excluded by!**/*.mp4docs/public/media/case-studies/flowmap-city.mp4is excluded by!**/*.mp4docs/public/media/case-studies/transcality.mp4is excluded by!**/*.mp4docs/public/media/examples/ai-example-dark.mp4is excluded by!**/*.mp4docs/public/media/examples/ai-example-light.mp4is excluded by!**/*.mp4docs/public/media/examples/multi-room.mp4is excluded by!**/*.mp4docs/public/media/examples/sqlrooms-ai-app-builder-2x.mp4is excluded by!**/*.mp4docs/public/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4is excluded by!**/*.mp4docs/public/media/key-concepts/room-shell.pngis excluded by!**/*.pngdocs/public/media/logo-large.pngis excluded by!**/*.pngdocs/public/media/offline/sqlrooms-query-pwa.pngis excluded by!**/*.pngdocs/public/media/overview/architecture.svgis excluded by!**/*.svgdocs/public/media/sqlrooms-examples-75.mp4is excluded by!**/*.mp4docs/public/media/sqlrooms-intro-2.mp4is excluded by!**/*.mp4docs/public/media/sqlrooms-query.mp4is excluded by!**/*.mp4docs/public/media/whats-new/commands.mp4is excluded by!**/*.mp4docs/public/media/whats-new/row-selection.pngis excluded by!**/*.pngdocs/public/media/whats-new/tab-strip-component.mp4is excluded by!**/*.mp4
📒 Files selected for processing (40)
.gitattributes.github/workflows/deploy-docs.yml.github/workflows/sync-examples.ymlCLAUDE.mdREADME.mddocs/public/media/case-studies/cosmograph.webpdocs/public/media/case-studies/flowmap-city.webpdocs/public/media/case-studies/fsq-spatial-desktop-earthquakes.webpdocs/public/media/case-studies/transcality.webpdocs/public/media/examples/ai-example.webpdocs/public/media/examples/cosmos-embedding-light.webpdocs/public/media/examples/cosmos-embedding.webpdocs/public/media/examples/cosmos-graph.webpdocs/public/media/examples/deckgl-discuss.webpdocs/public/media/examples/deckgl.webpdocs/public/media/examples/get-started.webpdocs/public/media/examples/kepler.webpdocs/public/media/examples/mosaic.webpdocs/public/media/examples/motherduck-example.webpdocs/public/media/examples/nextjs.webpdocs/public/media/examples/old-sqlrooms-ai.webpdocs/public/media/examples/rag.webpdocs/public/media/examples/sqlrooms-ai-example.webpdocs/public/media/examples/sqlrooms-query.webpdocs/public/media/overview/collage.webpexamples/ai-rag/README.mdexamples/ai/README.mdexamples/app-builder/README.mdexamples/cosmos-embedding/README.mdexamples/cosmos/README.mdexamples/deckgl-discuss/README.mdexamples/deckgl-mosaic/README.mdexamples/deckgl/README.mdexamples/kepler/README.mdexamples/mosaic/README.mdexamples/multi-room/README.mdexamples/nextjs/README.mdexamples/query-motherduck/README.mdexamples/query/README.mdscripts/update-readme.mjs
| This example is based on the [original demo app](https://github.com/dzole0311/deckgl-duckdb-geoarrow) by [Gjore Milevski](https://github.com/dzole0311). | ||
|
|
||
| <video src="/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4" alt="SQLRooms Deck.gl+Mosaic example app" width="450" controls loop muted></video> | ||
| <video src="https://sqlrooms.org/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4" alt="SQLRooms Deck.gl+Mosaic example app" width="450" controls loop muted></video> |
Contributor
There was a problem hiding this comment.
alt does not describe a <video> element.
The new hosted source is fine, but alt is ignored on <video>, so the description never reaches assistive tech. Use aria-label and/or fallback text instead.
♿ Suggested markup fix
-<video src="https://sqlrooms.org/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4" alt="SQLRooms Deck.gl+Mosaic example app" width="450" controls loop muted></video>
+<video
+ src="https://sqlrooms.org/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4"
+ aria-label="SQLRooms Deck.gl+Mosaic example app"
+ width="450"
+ controls
+ loop
+ muted
+>
+ SQLRooms Deck.gl+Mosaic example app preview video.
+</video>📝 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.
Suggested change
| <video src="https://sqlrooms.org/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4" alt="SQLRooms Deck.gl+Mosaic example app" width="450" controls loop muted></video> | |
| <video | |
| src="https://sqlrooms.org/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4" | |
| aria-label="SQLRooms Deck.gl+Mosaic example app" | |
| width="450" | |
| controls | |
| loop | |
| muted | |
| > | |
| SQLRooms Deck.gl+Mosaic example app preview video. | |
| </video> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/deckgl-mosaic/README.md` at line 11, Replace the invalid alt
attribute on the <video> element with accessible alternatives: add an aria-label
(and/or aria-labelledby) on the same <video> tag to provide a text description
for assistive tech, and include descriptive fallback text between the
<video>...</video> tags so non-supporting browsers and screen readers can access
the description; update the existing <video
src="https://sqlrooms.org/media/examples/sqlrooms-deckgl-mosaic-1500px.mp4" ...>
element accordingly.
Signed-off-by: Ilya Boyandin <[email protected]>
dmitriy-kostianetskiy
pushed a commit
that referenced
this pull request
Mar 9, 2026
Signed-off-by: Ilya Boyandin <[email protected]>
This was referenced Mar 9, 2026
mihajlo-stojkovic
pushed a commit
that referenced
this pull request
Mar 12, 2026
Signed-off-by: Ilya Boyandin <[email protected]>
bdjulbic
pushed a commit
that referenced
this pull request
Mar 16, 2026
Signed-off-by: Ilya Boyandin <[email protected]> Signed-off-by: bdjulbic <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR moves docs media to
docs/public/mediaand keeps it tracked with Git LFS, so the docs site can serve stable/media/...URLs without relying on VitePress’s hashed asset filenames. That lets us keep videos and images onsqlrooms.orgwhile avoiding GitHub repo-relative LFS links in the main repositoryREADME.It also standardizes media references by using relative
/media/...paths in the actual docs source, while keeping absolutehttps://sqlrooms.org/media/...URLs in standalone exampleREADMEs and the generated rootREADME.md. This preserves good local/docs behavior and reduces the chance of unnecessary Git LFS bandwidth usage from GitHub-hosted media rendering.Summary by CodeRabbit
Documentation
Chores