villebro commented on PR #44250:
URL: https://github.com/apache/superset/pull/44250#issuecomment-5751188320

   Picking up the three additional suggestions from Bito run #9b6078, all in 
`tests/unit_tests/scripts/docker_cache_ref_test.py`. Addressed in 5a30e8f995.
   
   **Suffix allowlist drift risk** — the real one. `PRESET_SUFFIXES` hard-coded 
`-dev`/`-lean`, but `scripts/docker-build-extra-flags.sh` appends 
`-${BUILD_PRESET}` from a generic case arm, so adding a matrix preset there 
would have left the allowlist quietly stale. Rather than couple the test to the 
script's preset list, it now matches the suffix generically 
(`(?:-[a-z0-9]+)?`): the invariant worth enforcing is the base Python version, 
not which presets happen to exist. Verified it still accepts `-dev`/`-lean` and 
a hypothetical `-py313`, still rejects a wrong base version 
(`3.12-slim-bookworm`), and rejects a malformed double suffix.
   
   **Missing helper and test docstrings** — added. Worth noting for future runs 
that the cited `BITO.md` rules (12147, 12148, 12490) are not discoverable in 
this repo; there is no `BITO.md` here, so those IDs only resolve inside the 
Bito workspace. The changes stand on `CLAUDE.md`'s own "Docstrings: Required 
for new functions/classes" instead. The local convention in that directory is 
also narrower than the cited rule — `change_detector_test.py` gives docstrings 
to helpers and to tests whose rationale is non-obvious, while leaving 
self-describing test names bare — so please read a bare test name there as 
house style rather than an omission.
   
   Re-verified after the change: 5 passed, and the negative check (temporarily 
bumping `ARG PY_VER` in the Dockerfile) still fails all five consumers.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to