Fix three review defects in the retro recipes and lint them in CI - #20
Conversation
Reviewing the two recipes just added found three defects, two of which would only have appeared on the Raspberry Pi they exist for. The frontend recipe passed --disable-wayland. That flag was chosen against a host where Wayland libraries were absent anyway, so it changed nothing during testing, but Raspberry Pi OS defaults to a Wayland session and forcing it off could leave the frontend unable to present in the operator's actual session. configure already detects Wayland and disables it on its own when the libraries are missing, so the flag is removed and detection is left to configure. The frontend recipe also required libgl1-mesa-dev by name. A Raspberry Pi provides GLES rather than desktop GL, so the precheck would have refused to run on the exact target this recipe was written for. GPU headers are now an any-of check across libgl1-mesa-dev and libgles2-mesa-dev, and the error message names which package belongs to which target. The precheck is also skipped with a notice on hosts without dpkg-query instead of failing, since configure is the real authority there. The cores recipe keyed a hard-coded zlib probe off a generic dependency field, so any future core declaring a different dependency would have been checked for the wrong library and then failed confusingly at its link step. The check now dispatches on the declared name and refuses with an explicit message when no probe exists for it. Verified by rewriting the field to an unknown package: the recipe reports the dependency by name and skips rather than building. Adds shellcheck over scripts/pi/*.sh to the pi bring-up job. The retro build recipes are deliberately not run in continuous integration because they depend on upstream archive availability, but linting them needs no network and is deterministic, so a quoting or word-splitting defect cannot reach the Pi unreviewed. All three scripts are clean at --severity=style. Re-verified after the fixes: shellcheck clean, the cores recipe still produces aarch64 objects for two cores and skips snes9x with the correct reason, and the frontend still builds and reports Version 1.22.2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
1 participant
Reviewing the two recipes just added found three defects, two of which would only have appeared on the Raspberry Pi they exist for.
1.
--disable-waylandforced off (frontend)Chosen against a host where Wayland libraries were absent anyway, so it changed nothing in testing. But Raspberry Pi OS defaults to a Wayland session, and forcing it off could leave the frontend unable to present in the operator's actual session.
configurealready detects and disables it when absent, so the flag is removed.2.
libgl1-mesa-devrequired by name (frontend)A Pi provides GLES, not desktop GL — so the precheck would have refused to run on the exact target this recipe exists for. GPU headers are now an any-of check across
libgl1-mesa-devandlibgles2-mesa-dev, with the error naming which belongs to which target. The precheck also degrades to a notice on non-dpkg hosts rather than failing.3. Generic dependency field, hard-coded zlib probe (cores)
Any future core declaring a different dependency would have been checked for the wrong library, then failed confusingly at link. The check now dispatches on the declared name. Verified by rewriting the field to an unknown package:
shellcheck in CI
The build recipes are deliberately not run in CI (upstream archive availability), but linting needs no network and is deterministic. All three scripts are clean at
--severity=style.Re-verified after the fixes
shellcheck clean; cores recipe still produces aarch64 objects for two cores and skips snes9x with the correct reason; frontend still builds and reports
Version: 1.22.2.🤖 Generated with Claude Code