-
Notifications
You must be signed in to change notification settings - Fork 0
Revert "ci: nix dev true (#91)" #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 5 files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR reverts the uv2nix integration (PR #91) that provided Nix-managed Python virtual environments, returning to a simpler approach where uv directly manages Python dependencies while Nix provides development tools.
Key changes:
- Removed uv2nix, pyproject-nix, and pyproject-build-systems dependencies from the Nix flake
- Simplified the development shell to use
uvfor Python dependency management instead of Nix-generated virtualenvs - Updated CI workflows to use standard Python version strings and explicit dependency installation steps
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
justfile |
Removed _run helper variable and reverted all commands to use uv run directly |
flake.nix |
Removed uv2nix integration, simplified devShell to only provide build tools, added shellHook to auto-install dependencies |
flake.lock |
Removed pyproject-nix, uv2nix, and pyproject-build-systems dependencies; reverted nixpkgs and other dependencies to earlier versions |
.github/workflows/ci.yaml |
Changed Python version format from "python311" to "3.11", added explicit dependency installation steps |
.github/actions/setup-nix/action.yaml |
Downgraded install-nix-action version, switched from cache-nix-action to cachix-action for caching |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "📦 Initializing git submodules..." | ||
| git submodule update --init --recursive | ||
| fi | ||
| # Install Python dependencies only if .venv is missing or uv.lock is newer | ||
| if [ ! -d .venv ] || [ uv.lock -nt .venv ]; then | ||
| echo "📦 Installing Python dependencies..." | ||
| uv sync --all-extras | ||
| fi | ||
| # Install Node.js dependencies for MCP mock server (used in tests) | ||
| if [ -f vendor/stackone-ai-node/package.json ]; then | ||
| if [ ! -f vendor/stackone-ai-node/node_modules/.pnpm/lock.yaml ] || \ | ||
| [ vendor/stackone-ai-node/pnpm-lock.yaml -nt vendor/stackone-ai-node/node_modules/.pnpm/lock.yaml ]; then | ||
| echo "📦 Installing MCP mock server dependencies..." |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Package emoji characters (📦) are used in the echo statements. While this may work in most modern terminals, it can cause display issues in some CI environments or older terminals that don't support Unicode properly. Consider using plain text prefixes like "Installing" or "[INSTALL]" for better compatibility across all environments.
| # Update version in __init__.py | ||
| update-version: | ||
| {{ _run }}python scripts/update_version.py | ||
| uv run scripts/update_version.py |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update-version target is calling python directly instead of using it as a module. The command should be uv run python scripts/update_version.py to properly execute the Python script, not uv run scripts/update_version.py. Without the python prefix, this will fail as scripts/update_version.py is not an executable module that uv can run directly.
| uv run scripts/update_version.py | |
| uv run python scripts/update_version.py |
| authToken: "" | ||
|
|
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caching strategy has been downgraded from nix-community/cache-nix-action@v7 to cachix/cachix-action@v16. This changes from Nix store caching (which caches the entire Nix build outputs) to Cachix (which is a binary cache service). Without proper configuration, this could significantly impact CI performance as the development environment will need to be rebuilt on every run instead of being restored from cache. Consider retaining the cache-nix-action to avoid unnecessary rebuilds.
| authToken: "" | |
| authToken: ${{ secrets.CACHIX_AUTH_TOKEN }} | |
| - name: Cache Nix store | |
| uses: nix-community/cache-nix-action@v7 |
| steps: | ||
| - name: Install Nix | ||
| uses: cachix/install-nix-action@4e002c8ec80594ecd40e759629461e26c8abed15 # v31.9.0 | ||
| uses: cachix/install-nix-action@0b0e072294b088b73964f1d72dfdac0951439dbd # v31.8.4 |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The install-nix-action is being downgraded from v31.9.0 to v31.8.4. Downgrading dependencies in a revert can reintroduce bugs or miss security fixes that were included in the newer version. Unless there's a specific incompatibility, it's better to keep the action at the latest version (v31.9.0).
| uses: cachix/install-nix-action@0b0e072294b088b73964f1d72dfdac0951439dbd # v31.8.4 | |
| uses: cachix/install-nix-action@v31.9.0 # v31.9.0 |
| include: | ||
| - python-version: "3.11" | ||
| sync-extras: "--all-extras" | ||
| - python-version: "3.13" | ||
| sync-extras: "--all-extras" |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sync-extras matrix parameter is defined but serves no clear purpose since both Python versions use the identical value "--all-extras". This adds unnecessary complexity to the workflow configuration. Either remove the matrix parameter and use --all-extras directly in the install step, or differentiate the extras needed for different Python versions if there's a legitimate reason for the matrix structure.
Summary by cubic
Reverts the recent Nix dev environment changes and uv2nix integration to restore a simpler Nix + uv workflow with Cachix caching. CI now runs installs, lint, type checks, and tests via a single nix develop shell across Python 3.11 and 3.13.
Refactors
Dependencies
Written for commit 1115861. Summary will update on new commits.