feat(bazaar): robust legacy schema support, instant local previews, and Freelens recommendations - #805
feat(bazaar): robust legacy schema support, instant local previews, and Freelens recommendations#805castrojo wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughThis PR changes the djxl color-space flag to a long-form option, adds a conditional JXL-to-PNG banner conversion step in two Justfile recipes, switches bazaar.service to Type=simple, adds an appid to curated.yaml, rewrites bazaar skill docs, broadens OCI tag-skipping logic with a new test, and isolates test PATH setup. ChangesBazaar banner conversion and service changes
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Just as bazaar-preview recipe
participant Podman
participant Filesystem as /etc/bazaar
Just->>Just: check branding dir exists
Just->>Podman: run alpine container with djxl
Podman->>Podman: convert *.jxl to *.png
Just->>Filesystem: sudo install generated PNGs
Just->>Just: remove temp directory
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
hanthor
left a comment
hanthor
left a comment
There was a problem hiding this comment.
Review: Approved ✅
Well-structured PR. Verified:
- djxl
-C→--color_space=sRGB— correct fix for newer libjxl - bazaar.service
Type=oneshot→Type=simple— correct for persistent daemon - Freelens added in correct spot in curated.yaml
bazaar-previewpodman-based preview workflows added to both Justfile and system.just- test_changelog.bats PATH hardened for test isolation
- docs/skills/bazaar.md v1.0→v1.1 rewrite with canonical format (version, tags, last_updated), Common Pitfalls, Red Flags, and Verification checklist
All 7 commits by castrojo use Conventional Commits with proper attribution.
🟡 Minor note: alpine:latest (no SHA) used in preview scripts — low risk for dev tooling, but SHA pinning would improve reproducibility.
|
Note for future selves, we'll replace all the containers like alpine with fsdk ones - there's so many everywhere that will take an audit but it unblocks us. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/skills/bazaar.md (1)
87-87: 📐 Maintainability & Code Quality | 🔵 TrivialMinor wording redundancy.
"blank/empty spaces" is redundant; "blank spaces" or "empty spaces" alone suffices.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/skills/bazaar.md` at line 87, The wording in the curated layout note is redundant because “blank/empty spaces” repeats the same idea. Update the sentence in the bazaar skills docs to use only one term, such as “blank spaces” or “empty spaces,” while keeping the PNG banner and /etc/bazaar context intact.Source: Linters/SAST tools
Justfile (1)
33-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated conversion logic between Justfile and system.just.
This ~15-line podman/djxl conversion block is copy-pasted almost verbatim into
system_files/bluefin/usr/share/ublue-os/just/system.just'sbazaar-preview. Consider extracting it into a shared script (e.g.scripts/bazaar-jxl-to-png.sh) invoked from both recipes to avoid drift when the conversion logic needs updating (as this PR itself just did for the-C/--color_spaceflag).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Justfile` around lines 33 - 49, The JXL-to-PNG conversion logic in the Justfile is duplicated elsewhere and will drift when updated. Extract the podman/djxl block from the Justfile into a shared script (for example, a bazaar conversion helper) and have both the current Justfile recipe and the bazaar-preview recipe in system.just call that shared helper so the conversion behavior stays consistent in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/skills/bazaar.md`:
- Line 87: The wording in the curated layout note is redundant because
“blank/empty spaces” repeats the same idea. Update the sentence in the bazaar
skills docs to use only one term, such as “blank spaces” or “empty spaces,”
while keeping the PNG banner and /etc/bazaar context intact.
In `@Justfile`:
- Around line 33-49: The JXL-to-PNG conversion logic in the Justfile is
duplicated elsewhere and will drift when updated. Extract the podman/djxl block
from the Justfile into a shared script (for example, a bazaar conversion helper)
and have both the current Justfile recipe and the bazaar-preview recipe in
system.just call that shared helper so the conversion behavior stays consistent
in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ed50033-e2af-4eeb-8850-0e134ef0d863
📒 Files selected for processing (9)
ContainerfileJustfiledocs/skills/bazaar.mdscripts/check-oci-refs.pysystem_files/bluefin/etc/bazaar/curated.yamlsystem_files/bluefin/usr/lib/systemd/user/bazaar.servicesystem_files/bluefin/usr/share/ublue-os/just/system.justtests/test_changelog.batstests/test_check_oci_refs.py
hanthor
left a comment
hanthor
left a comment
There was a problem hiding this comment.
This PR currently has merge conflicts with the base branch and can't be merged as-is. Could you rebase / resolve the conflicts? Happy to re-review once it's mergeable — the change itself looks reasonable.
- Replaced djxl `-C sRGB` with `--color_space=sRGB` to fix missing content in PNG conversions. - Changed bazaar.service `Type=oneshot` to `Type=simple` and removed `RemainAfterExit=yes` to prevent systemctl from hanging indefinitely on startup. - Cleaned the PATH in test_changelog.bats setup() to isolate tests from host-installed bctl, unblocking local development test runs. Assisted-by: Gemini 3.5 Flash via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Integrated automated podman-based JXL-to-PNG conversion in `just bazaar-preview` and `ujust bazaar-preview`. - This ensures host preview displays populated banners in io.github.kolunmi.Bazaar without requiring local host-installed `djxl`. Assisted-by: Gemini 3.5 Flash via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Documented sRGB conversion guidelines and the podman-based local conversion loop. - Specified Type=simple and no-window daemon lifecycle constraints for systemd integration. - Standardized file triggers, pitfalls, and verification criteria against the canonical /addyosmani/agent-skills spec. Assisted-by: Gemini 3.5 Flash via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pin docker.io/library/alpine in the bazaar-preview podman containers to the same SHA already used by the Containerfile build stage. Assisted-by: Claude Sonnet via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
74c7651 to
d8cbfa7
Compare
|
@hanthor rebased onto current origin/main, resolved the conflicts, and pinned the alpine preview image to the same SHA used in the Containerfile build stage. Ready for re-review — thanks! |
castrojo
added
4-review
2 participants
This PR resolves all longstanding Bazaar local development issues and integrates the new Freelens Kubernetes dashboard app:
-C sRGBflag with--color_space=sRGBto properly render converted banners with full colors and pixel data.bazaar.servicetype fromoneshottosimple(and removedRemainAfterExit=yes) so the background daemon immediately launches without blocking systemctl.podmanJXL-to-PNG decoding into the localjust bazaar-previewandujust bazaar-previewcommands, ensuring host previews are fully populated with banners.test_changelog.batsfailure path whenbctlis installed locally on the developer's host.app.freelens.FreelensKubernetes client into the curated Developers list incurated.yaml.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests