Use NIX for CI #113
Use NIX for CI #113
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a Nix-based development setup: a new Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
flake.nix (2)
21-24: Consider using a unified Rust toolchain instead of individual components.Installing Rust components individually (
rustc,cargo,rustfmt,clippy) may lead to version mismatches. The more idiomatic approach in Nix is to use a complete Rust toolchain package or a Rust overlay (likerust-binfrom oxalica).Consider this alternative approach:
tools = [ - pkgs.rustc - pkgs.cargo - pkgs.rustfmt - pkgs.clippy + pkgs.rustup pkgs.cargo-shear pkgs.cargo-sort pkgs.cargo-editAlternatively, if you want more control over the Rust version, consider using a Rust overlay like oxalica/rust-overlay.
5-6: Pin nixpkgs to a specific commit for reproducibility.Using
nixpkgs-unstablewithout a specific commit hash means different developers and CI runs may get different package versions over time, reducing build reproducibility.Consider pinning to a specific commit:
inputs = { - nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable"; + nixpkgs.url = "github:NixOS/nixpkgs?ref=nixpkgs-unstable"; flake-utils.url = "github:numtide/flake-utils"; };Then commit the
flake.lockfile (which should be auto-generated after runningnix flake update) to lock all input versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.envrc(1 hunks).github/workflows/pr.yml(2 hunks).gitignore(1 hunks)flake.nix(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
.envrc (1)
1-1: LGTM! Standard direnv + Nix flakes integration.The
use flakedirective correctly enables direnv to automatically load the development environment defined inflake.nixwhen entering the directory..gitignore (1)
8-9: LGTM! Correct direnv integration.Ignoring
.direnv/is appropriate for the direnv integration introduced in this PR..github/workflows/pr.yml (1)
19-31: LGTM! Clean Nix-based CI workflow.The workflow correctly:
- Installs Nix using the cachix action
- Validates the development environment with
nix develop- Sets RUSTFLAGS for the build
- Runs checks through the Nix dev shell
This approach ensures CI and local development environments stay in sync.
flake.nix (1)
1-40: No action required—flake.lock is already committed.The repository already contains
flake.lockand it is tracked by git, ensuring reproducible builds across environments. ✓
.gitignore
Outdated
| # Node.js | ||
| node_modules/ | ||
| dist/ |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Node.js usage in the project.
The .gitignore adds node_modules/ and dist/ (Node.js artifacts), and the CI workflow comment on line 24 of .github/workflows/pr.yml mentions "pnpm", but flake.nix doesn't include Node.js, npm, or pnpm in the tools list. Either these tools should be added to flake.nix, or these ignore patterns may be premature.
Run the following script to check if Node.js/npm/pnpm are referenced elsewhere in the project:
🏁 Script executed:
#!/bin/bash
# Check for Node.js/npm/pnpm usage in the project
echo "=== Checking for package.json ==="
fd -t f '^package\.json$'
echo -e "\n=== Checking for pnpm references ==="
rg -n 'pnpm' --type-add 'nix:*.nix' --type nix --type yaml
echo -e "\n=== Checking for node/npm references in justfile ==="
fd -t f '^[Jj]ustfile$' --exec cat {}Length of output: 3152
I need to verify the specific claims about pnpm and check the flake.nix configuration:
🏁 Script executed:
#!/bin/bash
# Check the CI workflow file for pnpm references
echo "=== Checking .github/workflows/pr.yml around line 24 ==="
if [ -f ".github/workflows/pr.yml" ]; then
sed -n '15,35p' .github/workflows/pr.yml
else
echo "File not found"
fi
echo -e "\n=== Checking for flake.nix ==="
if [ -f "flake.nix" ]; then
echo "flake.nix exists"
echo -e "\n=== Full flake.nix content ==="
cat flake.nix
else
echo "flake.nix not found"
fi
echo -e "\n=== Checking package.json content in web-transport-quinn/web/package.json ==="
if [ -f "web-transport-quinn/web/package.json" ]; then
head -20 web-transport-quinn/web/package.json
else
echo "File not found"
fiLength of output: 1632
Add Node.js tooling to flake.nix or remove Node.js references and gitignore entries.
The concern is valid: .github/workflows/pr.yml line 24 states "Use our flake to get all development tools (Rust, pnpm, just, cargo tools, etc.)" but flake.nix does not include nodejs, npm, or pnpm in its tools list. Meanwhile, package.json files exist in web-transport-quinn/web/ and web-transport-ws/ with Node.js dependencies (e.g., Parcel), making the node_modules/ and dist/ entries in .gitignore functional orphans—CI cannot build these packages since the Node.js toolchain is unavailable.
Either add pkgs.nodejs and pkgs.pnpm to the tools list in flake.nix, or remove the Node.js build artifacts from .gitignore and the misleading pnpm reference from the workflow comment.
🤖 Prompt for AI Agents
In .gitignore around lines 4 to 6, the repository ignores Node.js build
artifacts (node_modules/ and dist/) while flake.nix doesn't provide Node
toolchain, breaking CI; either update flake.nix to expose Node and pnpm (add
pkgs.nodejs and pkgs.pnpm to the tools/devShell so the workflow can install and
build JS packages) or remove Node.js references: delete node_modules/ and dist/
from .gitignore and remove/mend the pnpm mention in .github/workflows/pr.yml so
the workflow comment no longer promises Node tooling; pick one approach and make
corresponding changes across flake.nix and the workflow comment to keep tooling
and ignore rules consistent.
Summary by CodeRabbit