chore: add NVIDIA SPDX license header pre-commit hook - #422
Conversation
cmatzenbach
added
improvement
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds pre-commit configuration and dependency support for NVIDIA Apache-2.0 SPDX headers, then updates the license-check workflow to run ChangesCopyright checks
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)
19-28: 🔒 Security & Privacy | 🔵 TrivialRun this check in CI as well as developer-local pre-commit.
This configuration only enables a local hook; if the custom checker is being removed, ensure CI runs
pre-commit run verify-copyright --all-files. Otherwise contributors can bypass the check by not installing or skipping pre-commit. (pre-commit.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.pre-commit-config.yaml around lines 19 - 28, Ensure CI also runs the verify-copyright hook across all files, using the repository’s existing pre-commit configuration, while preserving the developer-local hook setup in .pre-commit-config.yaml.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 29-31: Update the files pattern used by verify-copyright to
preserve coverage for all tracked repository file types, including *.cpp, *.h,
CMakeLists.txt, and pyproject.toml; preferably remove the override to inherit
upstream coverage, or extend it with those types.
---
Nitpick comments:
In @.pre-commit-config.yaml:
- Around line 19-28: Ensure CI also runs the verify-copyright hook across all
files, using the repository’s existing pre-commit configuration, while
preserving the developer-local hook setup in .pre-commit-config.yaml.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: ba3dd184-821e-411b-ae32-7d25ade1f78c
📒 Files selected for processing (1)
.pre-commit-config.yaml
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟠 Other critical/major comments (1)
.pre-commit-config.yaml-34-37 (1)
34-37:⚠️ Potential issue | 🟠 MajorThe
filesoverride still skips trackedpyproject.toml.Because
pyproject.tomlmatches none of these alternatives,verify-copyright --all-fileswill not check it. Remove the override to inherit upstream coverage, or addpyproject\.toml$and any other required tracked file types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.pre-commit-config.yaml around lines 34 - 37, Update the pre-commit files filter to include pyproject.toml, or remove the override so upstream coverage applies; ensure verify-copyright --all-files checks this tracked file and other required file types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Other critical/major comments:
In @.pre-commit-config.yaml:
- Around line 34-37: Update the pre-commit files filter to include
pyproject.toml, or remove the override so upstream coverage applies; ensure
verify-copyright --all-files checks this tracked file and other required file
types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 92adfd37-e0d0-44f8-8735-a96172b4e264
📒 Files selected for processing (1)
.pre-commit-config.yaml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.pre-commit-config.yaml (1)
34-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore coverage for all tracked file types.
pre-commit run ... --all-filesstill applies thisfilesregex, so omitted extensions are never checked. The pattern appears to miss types such as.hpp,.cc,.cxx,.hh,.cu,.cuh,.cmake,CMakeLists_standalone.txt,.pyx,.pxd,pyproject.toml,meta.yaml, andsetup.cfg. Remove the override if the hook defaults cover them, or extend it to match the repository’s complete tracked-file set.#!/usr/bin/env bash set -euo pipefail git ls-files | grep -E \ '(\.(cc|cxx|hh|hpp|cu|cuh|cmake|pyx|pxd)$|(^|/)(CMakeLists_standalone\.txt|meta\.yaml|pyproject\.toml|setup\.cfg)$)'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.pre-commit-config.yaml around lines 34 - 38, Update the files regex in the pre-commit configuration so all tracked source and configuration types are covered during --all-files runs. Prefer removing the custom files override if the hook defaults provide complete coverage; otherwise extend the pattern to include .hpp, .cc, .cxx, .hh, .cu, .cuh, .cmake, .pyx, .pxd, CMakeLists_standalone.txt, meta.yaml, pyproject.toml, and setup.cfg while preserving existing matches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.pre-commit-config.yaml:
- Around line 34-38: Update the files regex in the pre-commit configuration so
all tracked source and configuration types are covered during --all-files runs.
Prefer removing the custom files override if the hook defaults provide complete
coverage; otherwise extend the pattern to include .hpp, .cc, .cxx, .hh, .cu,
.cuh, .cmake, .pyx, .pxd, CMakeLists_standalone.txt, meta.yaml, pyproject.toml,
and setup.cfg while preserving existing matches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 4dc4a55b-ddd4-45b5-8815-b1a78f58c51a
📒 Files selected for processing (2)
.github/workflows/license-check.yml.pre-commit-config.yaml
| - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 | ||
| with: | ||
| python-version: "3.x" | ||
| - name: Check SPDX license headers | ||
| run: | | ||
| # ts-bindings are auto-generated by ts-rs from Rust structs | ||
| missing=() | ||
| while IFS= read -r file; do | ||
| if ! grep -q "SPDX-License-Identifier" "$file"; then | ||
| missing+=("$file") | ||
| fi | ||
| done < <(git ls-files -- '*.ts' '*.tsx' '*.rs' '*.proto' '*.css' \ | ||
| ':!examples/simulator/server/ts-bindings/') | ||
|
|
||
| if [[ ${#missing[@]} -gt 0 ]]; then | ||
| echo "The following files are missing an SPDX license header:" | ||
| printf ' %s\n' "${missing[@]}" | ||
| echo "" | ||
| echo "Add the following header to the top of each file:" | ||
| echo " // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." | ||
| echo " // SPDX-License-Identifier: Apache-2.0" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "All $(git ls-files -- '*.ts' '*.tsx' '*.rs' '*.proto' '*.css' \ | ||
| ':!examples/simulator/server/ts-bindings/' | wc -l | tr -d ' ') files have SPDX license headers." | ||
| pip install pre-commit | ||
| pre-commit run verify-copyright --all-files |
There was a problem hiding this comment.
For consistency with the rest of the checks, let's use Pixi.
This lets us pin pre-commit through its lock file for maximum reproducibility.
There was a problem hiding this comment.
Updated to use pixi instead of pip
| fi | ||
|
|
||
| echo "All $(git ls-files -- '*.ts' '*.tsx' '*.rs' '*.proto' '*.css' | wc -l | tr -d ' ') files have SPDX license headers." | ||
| run: pixi run pre-commit run verify-copyright --all-files |
There was a problem hiding this comment.
Complains about a misplaced comma in some files, otherwise looking great --> 2026, NVIDIA vs 2026 NVIDIA
-// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES.
All rights reserved.
+// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES.
All rights reserved.
johallar
left a comment
johallar
left a comment
There was a problem hiding this comment.
Running the pre-commit command auto fixes the header typos, then all good!
|
/merge |
3 participants
Description
Adds a .pre-commit-config.yaml that uses the rapidsai/pre-commit-hooks verify-copyright hook to automatically insert and maintain NVIDIA SPDX license headers on commit.
Related Issues
Closes #224
Testing
pre-commit run verify-copyright --all-filesreports no changes