Skip to content

update hooks, add dco check#16

Open
zsblevins wants to merge 1 commit into
mainfrom
update-hooks
Open

update hooks, add dco check#16
zsblevins wants to merge 1 commit into
mainfrom
update-hooks

Conversation

@zsblevins
Copy link
Copy Markdown
Collaborator

@zsblevins zsblevins commented May 29, 2026

Description

  • Update list of dirs for format check
  • Add commit-msg hook for DCO check

Validation

zblevins@omni-lfn-dg82r:~/git/nv-config-manager$ git commit -s -m 'update hooks, add dco check' .
Running ruff format on staged Python files...
      Built nv-config-manager @ file:///home/zblevins/git/nv-config-manager
Uninstalled 2 packages in 95ms
Installed 2 packages in 63ms
1 file reformatted, 1 file left unchanged
✓ Formatted 2 Python file(s)
Checking SPDX license headers...
✓ All 2 checked files have valid SPDX headers
✓ DCO sign-off present

Checklist

  • I am familiar with the contributing guidelines in CONTRIBUTING.md.
  • Commits are signed off for DCO compliance.
  • New or existing tests cover these changes, or the PR explains why tests are not needed.
  • Documentation is updated for user-facing behavior changes.
  • Generated artifacts are updated when applicable, such as OpenAPI specs,
    docs screenshots, or Helm/rendered outputs.

Summary by CodeRabbit

  • Documentation

    • Expanded contributing guidelines and setup documentation with a dedicated Git hooks section, detailing hook behaviors, installation steps, and instructions for bypassing hooks locally.
  • Chores

    • Implemented Git hooks enforcing Python code formatting compliance and Developer Certificate of Origin (DCO) sign-off requirements on commits.
    • Updated installation scripts to support automatic setup of multiple hooks.

Review Change Stack

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 29, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

This PR introduces a comprehensive Git hooks infrastructure for developer certificate and code quality enforcement. It adds two new Git hooks for automated validation, creates an installation mechanism, extends SPDX header processing to additional file types, and documents the setup in development guides.

Changes

Git Hooks Infrastructure

Layer / File(s) Summary
commit-msg and pre-commit hook scripts
scripts/hooks/commit-msg, scripts/hooks/pre-commit
New commit-msg hook enforces DCO Signed-off-by trailers with valid email patterns while rejecting placeholders. Refactored pre-commit hook collects staged files via null-delimited read and formats staged Python files with ruff before validating SPDX headers across allowed directories with updated extension support.
Hook installation script
scripts/install-hooks.sh
Updated to iterate over both pre-commit and commit-msg hooks, copying from scripts/hooks/ to .git/hooks/, marking executable, and failing if any hook file is missing. Output messages reflect both installed hooks and provide guidance for skipping hooks locally.
Development setup and contribution documentation
README.md, CONTRIBUTING.md
README Local Development Setup adds dedicated "Git Hooks" subsection with installation instructions, hook behavior descriptions, and bypass guidance. Contributing workflow steps insert hook installation before tests/lint. CONTRIBUTING.md expands to detail pre-commit Python formatting and SPDX validation plus commit-msg DCO enforcement.
SPDX header processing extension
scripts/add_spdx_headers.py
Extended to include installer/tests and installer/scripts directories for Python header processing. JavaScript/TypeScript processing expanded to include .js, .jsx, .mjs, .cjs extensions alongside .ts and .tsx.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 Hooks and safeguards align,
Staged files format with delight,
DCO trailers sign,
SPDX headers shine so bright,
Code quality takes flight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'update hooks, add dco check' accurately describes the main changes: updating hook scripts and adding DCO enforcement via commit-msg hook, which are the primary alterations across the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch update-hooks

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Zoe Blevins <zblevins@nvidia.com>
Copy link
Copy Markdown
Collaborator

@polarweasel polarweasel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering about the need for full license text (in all files, really), but the changes look good 👍

Comment thread scripts/hooks/commit-msg
# SPDX-FileCopyrightText: Copyright (c) 2024-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the full license text here when it's in a separate file also at the repo root?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants