Pin the setup venv to python 3.12 and ignore it - #118
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e396422ba1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
andiwand
force-pushed
the
modernize/10-readme-setup
branch
from
July 26, 2026 08:49
e396422 to
48939db
Compare
conan installs happily on python 3.11, and then conan/setup-all.sh calls the conan-odr-index helper scripts and dies on a SyntaxError, because those use PEP 701 nested quotes in f-strings. CI already pins 3.12 for exactly this reason; only the README was silent about it. The documented venv lives inside the checkout, so .venv/ joins .gitignore. Without it, following the setup top to bottom leaves every clone with a dirty tree and a large machine-specific directory one `git add .` away from being committed. The step reordering this branch also carried is now redundant: #113 moved the submodule checkout ahead of the pip install while landing SPM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019978mDv7veoaSpByQPCnNe
andiwand
force-pushed
the
modernize/10-readme-setup
branch
from
July 26, 2026 08:51
48939db to
d8d9f47
Compare
andiwand
changed the title
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
1 participant
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.
Follow-up to #108. Rebased onto main now that #113 has landed, which absorbed part of this branch — see the last section.
The venv had no version floor. conan installs happily on python 3.11, and then
conan/setup-all.shcalls the index's helper scripts and dies:PEP 701 nested quotes, so 3.12 is the floor. CI already pins
python-version: "3.12"for exactly this reason — only the README was silent about it.Verified by compiling the submodule's scripts under both interpreters: 3.11 raises the error above, 3.12 is clean.
.venv/is now ignored. Codex flagged this on the first revision and it's right: the step above creates the venv inside the checkout, and nothing in.gitignorematched it, so following the documented setup left a dirty tree with a large machine-specific directory sitting ingit status.What the rebase dropped. This branch also reordered the steps so the submodule checkout precedes the
pip installthat readsconan-odr-index/requirements.txt. #113 made that same reordering while replacing CocoaPods with SPM, so the rebase resolved to just the python floor plus the.gitignoreentry. The remaining README diff is 12 lines.🤖 Generated with Claude Code
https://claude.ai/code/session_019978mDv7veoaSpByQPCnNe