main

Nixpkgs PR Review Checklist

For AI Code Reviews

When performing automated PR review, prioritize checking these items in order:

Priority 0 (Blocking - Must Fix Before Merge):

  1. finalAttrs pattern used (not rec) - see Code Quality Checks section 1 below
  2. All required meta attributes present: description, homepage, license, maintainers
  3. mainProgram specified for CLI tools
  4. SRI hash format (hash = "sha256-..." not old-style sha256 = "...")

Priority 1 (Should Fix): 5. All version references use finalAttrs.version (in src.rev, ldflags, changelog) 6. Dependencies correctly categorized (nativeBuildInputs vs buildInputs) 7. Tests are enabled (doCheck not disabled without good reason)

Priority 2 (Nice to Have): 8. Changelog URL includes finalAttrs.version reference 9. Package structure follows pkgs/by-name convention 10. passthru.tests included where applicable

Before Review - Building and Testing:

Run this command to build and test the changes:

nixpkgs-review pr <number> --approve-pr --print-result

The --approve-pr flag marks the PR as reviewed, and --print-result shows which packages built successfully. Include the build results in your review context - they help verify the package actually works.

Review Output Format:

  • List findings with clear priority tags: [P0], [P1], [P2]
  • Include file path and line number for each finding
  • Provide \``suggestion` blocks for fixes where applicable
  • Reference build output when discussing functionality
  • End with verdict: “Ready to merge” or “Needs attention” with summary

General Review Process

  1. Check CI Status

    • All ofborg checks passing
    • GitHub Actions passing
    • No eval errors

  2. Build and Test

    nixpkgs-review pr <number> --approve-pr --print-result

    The --approve-pr flag marks the PR as reviewed after successful build. The --print-result flag shows which packages were built and tested.

  3. Verify the package works

    • Test --version flag
    • Test --help output
    • Run basic functionality tests

  4. Merge Bot (for Maintainers)

    • If PR has label 2.status: merge-bot eligible, you can request automated merge
    • Command: @NixOS/nixpkgs-merge-bot merge (in PR comment)
    • Requirements:
      • You are listed as maintainer of all touched packages
      • PR targets a development branch (master, staging, etc.)
      • PR touches only files in pkgs/by-name/
      • PR is approved OR opened by r-ryantm/committer
    • Useful for simple version bumps and dependency updates
    • Note: Sometimes GitHub gets stuck - leave another approval to trigger merge

Code Quality Checks

1. Use finalAttrs Pattern (CRITICAL)

Why: Ensures proper behavior with overrideAttrs and self-referential packages.

Bad:

buildGoModule rec {
  pname = "example";
  version = "1.0.0";
  
  src = fetchFromGitHub {
    rev = "v${version}";  # ❌ Uses 'rec' binding
  };
  
  ldflags = [ "-X main.version=v${version}" ];  # ❌ Won't update on override
}

Good:

buildGoModule (finalAttrs: {
  pname = "example";
  version = "1.0.0";
  
  src = fetchFromGitHub {
    rev = "v${finalAttrs.version}";  # ✅ Uses finalAttrs
  };
  
  ldflags = [ "-X main.version=v${finalAttrs.version}" ];  # ✅ Properly updates
})

What to check:

  • ✅ Function signature: buildGoModule (finalAttrs: { ... })
  • ✅ All self-references use finalAttrs.attribute not rec
  • ✅ Version in URLs: "v${finalAttrs.version}"
  • ✅ Version in ldflags: finalAttrs.version
  • ✅ Changelog URLs: finalAttrs.version

2. Package Structure

  • ✅ Located in pkgs/by-name/XX/package-name/package.nix
    • XX = first 2 letters of package name (lowercase)
  • ✅ File named package.nix (not default.nix)
  • ✅ Appropriate builder used:
    • buildGoModule for Go
    • buildPythonPackage for Python
    • buildRustPackage for Rust
    • etc.

3. Meta Attributes (REQUIRED)

meta = {
  description = "Clear, concise description";  # ✅ Required
  homepage = "https://example.com";             # ✅ Required
  changelog = "https://github.com/.../v${finalAttrs.version}";  # ✅ Recommended
  license = lib.licenses.mit;                   # ✅ Required
  maintainers = with lib.maintainers; [ username ];  # ✅ Required
  mainProgram = "binary-name";                  # ✅ Required for CLIs
  platforms = lib.platforms.unix;               # ✅ Specify supported platforms
};

What to check:

  • ✅ Description is accurate (verify against upstream)
  • ✅ License matches upstream (check LICENSE file)
  • ✅ Homepage URL is correct
  • ✅ Changelog link uses finalAttrs.version
  • ✅ Maintainer is added to maintainer list
  • ✅ mainProgram specified for CLI tools

4. Source Fetching

For GitHub releases:

src = fetchFromGitHub {
  owner = "user";
  repo = "repo";
  rev = "v${finalAttrs.version}";  # ✅ Or tag/commit
  hash = "sha256-...";              # ✅ Use SRI hash
};

What to check:

  • ✅ Uses appropriate fetcher (fetchFromGitHub, fetchurl, etc.)
  • ✅ Hash is correct and uses SRI format (sha256-...)
  • ✅ rev or tag references finalAttrs.version
  • ❌ No hardcoded versions in URLs

5. Dependencies

nativeBuildInputs = [ ];  # Build-time tools (pkg-config, cmake, etc.)
buildInputs = [ ];        # Runtime libraries
nativeCheckInputs = [ ];  # Test-time tools

What to check:

  • ✅ Dependencies properly categorized
  • ✅ No unnecessary dependencies
  • ✅ Runtime dependencies are correct (ldd check for binaries)

6. Version Handling

For Go modules:

buildGoModule (finalAttrs: {
  ldflags = [
    "-s"
    "-w"
    "-X github.com/user/repo/version.Version=v${finalAttrs.version}"
  ];
})

What to check:

  • ✅ Version embedded correctly
  • ✅ vendorHash is specified and correct
  • ✅ Tests are enabled (nativeCheckInputs if needed)

7. Tests

What to check:

  • ✅ Upstream tests run during build
  • ✅ doCheck is not disabled without reason
  • ✅ If tests require additional tools, they’re in nativeCheckInputs
  • ✅ Package includes passthru.tests if applicable

Init PR Specific Checks

For new packages (init at X.Y.Z), additionally verify:

1. Package Readiness

  • ✅ Package is stable (not alpha/beta)
  • ✅ Clear license statement
  • ✅ Actively maintained upstream
  • ✅ Will be used by others (check GitHub stars/issues)

2. Version Currency

  • ✅ Latest stable version is packaged
  • 🔍 Check: curl -s https://api.github.com/repos/USER/REPO/releases/latest

3. Comprehensive Testing

  • ✅ Actually run the binary
  • ✅ Test basic functionality
  • ✅ Check --version output matches
  • ✅ Verify help text appears

4. Security Considerations

  • ✅ No vendored outdated dependencies
  • ✅ Built from source (not binary-only)
  • ✅ No known vulnerabilities

Common Issues

1. Missing finalAttrs

Issue: Using rec instead of finalAttrs Fix: Convert to buildGoModule (finalAttrs: { ... }) pattern

2. Hardcoded Versions

Issue: rev = "v1.0.0" instead of rev = "v${finalAttrs.version}" Fix: Use finalAttrs.version for all version references

3. Wrong License

Issue: License doesn’t match upstream Fix: Check upstream LICENSE file, update meta.license

4. Missing mainProgram

Issue: CLI tool without mainProgram Fix: Add mainProgram = "binary-name"; to meta

5. Incorrect Dependencies

Issue: Build-time dependencies in buildInputs Fix: Move to nativeBuildInputs

Review Comment Templates

Request finalAttrs Pattern

Please use the `finalAttrs` pattern for proper override behavior:

\`\`\`suggestion
buildGoModule (finalAttrs: {
\`\`\`

And update all version references:

\`\`\`suggestion
  rev = "v${finalAttrs.version}";
\`\`\`

Request Version Update

Upstream has released v X.Y.Z. Consider updating before merging.

https://github.com/USER/REPO/releases/latest

Approve with Minor Suggestion

Looks good! Built and tested successfully with `nixpkgs-review pr XXXXX`.

One small thing: [suggestion]

Either way, this is ready to go. Nice work! 👍

Reference Documentation