Repository navigation
Conversation
unit.yaml runs tox -e py3-unit across a matrix.python of 3.11/3.12/3.13, but [testenv:py3-unit] pinned basepython to python3.11, silently re-pinning every leg back to 3.11 regardless of what setup-python installed. Removing the pin lets py3-unit use whatever interpreter is already on PATH for that job, so the three CI legs actually diverge. py3-smoke keeps its own basepython pin unchanged. Ref: instructlab#705 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Kush Zingade <kush.zingade@gmail.com>
|
No actionable com 8000 ments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe ChangesTox interpreter selection
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The change allows each py3-unit CI matrix job to use its configured Python version while leaving other environments unchanged; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
All four of these (#712, #713, #714, #715) are mergeable with zero human reviews — and CodeRabbit rate-limited on three of them, so they've had no review at all. Since the constraint looks like review bandwidth rather than the changes themselves: want me to consolidate the four into a single PR so it's one review pass instead of four? They're independent commits, so either shape works — say the word and I'll have it up the same day. |
|
Friendly ping @RobotSail @Maxusmusti on this and the three related small PRs from the same pass: #713 and #715 (docs corrections) and #714 (the unreachable DeepSpeed+cpu_offload xfail). All four are green and mergeable. Happy to squash them into one if that is easier to review. |
|
@RobotSail @Maxusmusti one last check-in on #712–#715. All four are still mergeable and small (a tox pin, two doc corrections, and an unreachable xfail). If the repo isn't taking changes right now, let me know and I'll close them. |
|
Tick the box to add this pull request to the merge queue (same as
|
|
Hi @UgaTheDev, thanks for opening these PRs, and apologies for the delay! This repo is currently transitioning into maintenance mode, but I will take a look at the PRs you mentioned |
unit.yamlrunstox -e py3-unitacross amatrix.pythonof3.11,3.12,and
3.13(three separate jobs, each withactions/setup-pythoninstallingthe matching interpreter). But
[testenv:py3-unit]intox.inisetsbasepython = python3.11, sotoxre-pins every leg back to 3.11 regardlessof which interpreter
setup-pythonput onPATH. All three matrix legscurrently exercise the same interpreter — the 3.12/3.13 legs are decorative.
This removes that pin. With no
basepythonoverride,tox'suv-venv-runneruses the interpreter already on
PATHfor that job — i.e. whateveractions/setup-pythonset up for the current matrix leg — so the three CIjobs actually diverge.
Scope is intentionally narrow: only
py3-unit.py3-smoke(tox.iniline 53)keeps its
basepython = python3.11pin — it isn't part of the version matrixin
smoke.yaml/smoke-py312.yamland shouldn't silently start floating.Verified locally:
tox -lstill listspy3-unitwith the same commands;only the interpreter selection changes. No other env is touched.
Originally flagged in #705 (comment from 2026-07-29): "the 3.11/3.12/3.13 matrix is decorative:
[testenv:py3-unit]intox.inisetsbasepython = python3.11, so tox re-pins the interpreter to 3.11 on all three legs no matter whatsetup-pythoninstalled."Fixes the tox side of the CI-matrix finding in #705.
Summary by CodeRabbit