From c0633b7c8d872717b1b079ed9b71f43198c8b7a2 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang Date: Thu, 1 Oct 2026 12:26:43 -0700 Subject: [PATCH 1/2] chore(contributing): align contribution conventions Signed-off-by: Jingxiang Zhang --- .git-hooks/commit-msg | 53 +++++++-------- .github/PULL_REQUEST_TEMPLATE.md | 12 +++- .../workflows/contribution-conventions.yml | 26 +++++++ CONTRIBUTING.md | 60 +++++++++++++---- Makefile | 4 ++ scripts/test-commit-conventions.sh | 67 +++++++++++++++++++ 6 files changed, 179 insertions(+), 43 deletions(-) create mode 100644 .github/workflows/contribution-conventions.yml create mode 100755 scripts/test-commit-conventions.sh diff --git a/.git-hooks/commit-msg b/.git-hooks/commit-msg index 202f449..320298f 100755 --- a/.git-hooks/commit-msg +++ b/.git-hooks/commit-msg @@ -2,40 +2,37 @@ # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 - set -eu -MSG_FILE="$1" - -# First non-empty, non-comment line is the commit subject. -SUBJECT=$(grep -vE '^(#|$)' "$MSG_FILE" | head -n1) +# --subject validates a PR title strictly; file mode is Git's commit-msg hook. +if [ "${1:-}" = "--subject" ]; then + [ "$#" -eq 2 ] || exit 1 + subject=$2 +else + [ "$#" -eq 1 ] || exit 1 + subject=$(sed '/^[[:space:]]*$/d; /^#/d' "$1" | head -n 1) + # Allow Git-generated subjects and temporary rebase commits locally. + case "$subject" in + "Merge "*|"Revert "*|"fixup! "*|"squash! "*|"amend! "*) exit 0 ;; + esac +fi -# Skip subjects generated by git. -case "$SUBJECT" in - "Merge "*|"Revert "*|"fixup! "*|"squash! "*|"amend! "*) exit 0 ;; +# Titles and commit subjects must be a single line. +case "$subject" in + *' +'*) echo "Subject must be a single line." >&2; exit 1 ;; esac -# Conventional-commit format: -# (): -PATTERN='^(feat|fix|perf|refactor|docs|chore|test|revert)(\([a-z0-9._/-]+\))?!?: .+$' - -if ! printf '%s' "$SUBJECT" | grep -qE "$PATTERN"; then - cat >&2 <(): - +pattern='^(feat|fix|perf|refactor|docs|chore|test|revert)(\([a-z0-9._/-]+\))?!?: [^[:space:]].*$' +if ! printf '%s\n' "$subject" | LC_ALL=C grep -qE "$pattern"; then + cat >&2 <<'EOF' +Expected: (): +Scope is optional. Add ! after the type/scope for a breaking change. +Allowed types: feat, fix, perf, refactor, docs, chore, test, revert Examples: - feat(cli): add node list command - fix(sdk): handle empty service key - chore: update dependencies - -Allowed types: - feat, fix, perf, refactor, docs, chore, test, revert + feat(cli): add inventory command + fix: handle missing chassis + feat(api)!: remove deprecated endpoint EOF exit 1 fi diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 53c2300..61f828b 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,10 +1,20 @@ + ## Description +## Breaking changes + + + +## Validation + + + ## Checklist + - [ ] I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/fleet-intelligence-client/blob/main/CONTRIBUTING.md). -- [ ] My commits are signed off (`git commit -s`) per the [Contributing Guidelines](https://github.com/NVIDIA/fleet-intelligence-client/blob/main/CONTRIBUTING.md#signing-off-your-work). +- [ ] My commits are signed off (`git commit -s`) with my real name and valid email per the [Contributing Guidelines](https://github.com/NVIDIA/fleet-intelligence-client/blob/main/CONTRIBUTING.md#signing-off-your-work). - [ ] New or existing tests cover these changes. - [ ] The documentation is up to date with these changes. diff --git a/.github/workflows/contribution-conventions.yml b/.github/workflows/contribution-conventions.yml new file mode 100644 index 0000000..3027910 --- /dev/null +++ b/.github/workflows/contribution-conventions.yml @@ -0,0 +1,26 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +name: Contribution conventions + +on: + pull_request: + types: [opened, edited, synchronize, reopened, ready_for_review] + +permissions: + contents: read + +jobs: + pr-title: + name: PR title + runs-on: ubuntu-latest + steps: + - name: Check out code + uses: actions/checkout@v4 + with: + persist-credentials: false + - name: Test subject validation + run: sh scripts/test-commit-conventions.sh + - name: Validate PR title + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: sh .git-hooks/commit-msg --subject "$PR_TITLE" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b87e6a9..e7c52c9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -119,7 +119,8 @@ Additional labels must be applied to indicate whether the change is a feature, i ### Local Git hooks Local hooks are optional but recommended. They check commit subject formatting -and scan staged files for secrets before commit. +and scan staged files for secrets before commit. See the shared conventions +below for the subject format and PR-title validation. Install `trufflehog` before enabling the hooks: @@ -155,21 +156,52 @@ Release branches may be created for stabilization or hotfix work when needed. If a release branch exists, maintainers will document the target branch in the issue or release notes. -### Branch naming +## Commit and Pull Request Conventions -Branches used to create PRs should have a name of the form `/`, -where `` is one of: +Use the same format for commit subjects and pull request titles: -- `feat` for new features. -- `fix` for bugs or regressions. -- `docs` for documentation changes. -- `test` for test-only changes. -- `chore` for maintenance. +```text +(): +``` + +- Allowed types: `feat`, `fix`, `docs`, `test`, `refactor`, `perf`, `chore`, `revert`. +- Scope is optional and lowercase, for example `cli`, `inventory`, or `deps`. +- Write a concise, imperative description, for example `fix(inventory): handle missing chassis`. +- For breaking changes, add `!` after the type/scope, for example `feat(cli)!: remove deprecated flag`, and explain the impact and migration in the PR body. +- Sign off every commit with `git commit -s`, using your real name and a valid email address, to comply with the DCO. +- Name branches `/`, for example `fix/missing-chassis` or `docs/configuration-guide`. Use one of the allowed types above. +- Target `main` unless maintainers specify a release branch. + +PR descriptions must explain the behavior change, link related GitHub issues +(use `Closes #123` when appropriate), report validation and remaining limitations, +and highlight breaking changes. Add relevant tests and documentation, follow the +PR template, and ensure CI passes. PR titles describe the final user-visible +result and may appear in release notes. + +Maintainers should **squash merge** each focused PR, using the PR title as the +final commit subject. Contributors may keep separate review commits; they do not +need to squash before review. Preserve all contributors' DCO `Signed-off-by:` +trailers in the final squash commit and verify the final message before merging. + +### Local validation + +Install the optional commit-format hook from the repository root: -Use a short dash-separated name after the slash, for example `feat/node-tags` -or `docs/report-examples`. +```bash +make setup-git-hooks +``` -### Release notes +Run the shared subject-validation tests or check a proposed PR title: + +```bash +make test-commit-conventions +sh .git-hooks/commit-msg --subject "fix(inventory): handle missing chassis" +``` -Pull request titles are used to generate release notes. Make them concise and -descriptive of the user-visible change. +The hook allows Git-generated merge/revert subjects and temporary +`fixup!`, `squash!`, and `amend!` commits locally. PR titles must always follow +the format above. The **Contribution conventions / PR title** GitHub Actions check +runs when a PR is opened, edited, or updated. Maintainers should require that +check in branch protection and enable squash merging in repository settings. +The hook checks subject formatting; contributors remain responsible for DCO +sign-offs. diff --git a/Makefile b/Makefile index ec59250..7cd6dbe 100644 --- a/Makefile +++ b/Makefile @@ -95,3 +95,7 @@ clean: ## Remove local build artifacts .PHONY: help help: ## Show available targets @awk 'BEGIN {FS = ":.*##"} /^[a-zA-Z_-]+:.*##/ {printf "%-12s %s\n", $$1, $$2}' $(MAKEFILE_LIST) + +.PHONY: test-commit-conventions +test-commit-conventions: ## Validate commit subjects and PR titles + sh scripts/test-commit-conventions.sh diff --git a/scripts/test-commit-conventions.sh b/scripts/test-commit-conventions.sh new file mode 100755 index 0000000..431b4ce --- /dev/null +++ b/scripts/test-commit-conventions.sh @@ -0,0 +1,67 @@ +#!/bin/sh + +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +set -eu + +cd "$(dirname "$0")/.." +message_file=$(mktemp) +trap 'rm -f "$message_file"' EXIT HUP INT TERM + +accept() { + sh .git-hooks/commit-msg --subject "$1" + printf '# comment\n\n%s\n\nCommit body.\n' "$1" > "$message_file" + sh .git-hooks/commit-msg "$message_file" +} +reject() { + if sh .git-hooks/commit-msg --subject "$1" >/dev/null 2>&1; then + echo "Unexpectedly accepted title: $1" >&2 + exit 1 + fi + printf '%s\n' "$1" > "$message_file" + if sh .git-hooks/commit-msg "$message_file" >/dev/null 2>&1; then + echo "Unexpectedly accepted commit: $1" >&2 + exit 1 + fi +} + +for type in feat fix perf refactor docs chore test revert; do + accept "$type: update behavior" + accept "$type(cli): update behavior" + accept "$type!: change behavior" + accept "$type(api/v2)!: change behavior" +done +reject '' +reject 'add feature' +reject 'Feat: add feature' +reject 'build: update image' +reject 'fix(API): handle missing chassis' +reject 'fix:missing space' +reject 'fix: ' +reject 'fix: ' +reject 'fix: handle missing chassis' + +# Local rebase/merge exceptions must never bypass strict PR-title validation. +for subject in 'Merge branch main' 'Revert "old change"' 'fixup! fix: handle missing chassis' 'squash! feat: add feature' 'amend! docs: update guide'; do + printf '%s\n' "$subject" > "$message_file" + sh .git-hooks/commit-msg "$message_file" + if sh .git-hooks/commit-msg --subject "$subject" >/dev/null 2>&1; then + echo "Unexpectedly accepted Git-generated PR title: $subject" >&2 + exit 1 + fi +done + +: > "$message_file" +if sh .git-hooks/commit-msg "$message_file" >/dev/null 2>&1; then + echo "Unexpectedly accepted empty commit." >&2 + exit 1 +fi +if sh .git-hooks/commit-msg --subject 'fix: first line +fix: second line' >/dev/null 2>&1; then + echo "Unexpectedly accepted multiline title." >&2 + exit 1 +fi + +# Shell metacharacters are data, including in titles supplied through CI env. +accept 'fix(cli): handle $HOME, `commands`, and $(commands) literally' +echo "Contribution convention checks passed." From b1549a65e1a971b7fa0deda4abb7bd410f58ea6f Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang Date: Thu, 1 Oct 2026 13:10:37 -0700 Subject: [PATCH 2/2] chore(contributing): simplify PR title validation Signed-off-by: Jingxiang Zhang --- .git-hooks/commit-msg | 53 ++++++++------- .../workflows/contribution-conventions.yml | 14 ++-- CONTRIBUTING.md | 31 +++------ Makefile | 4 -- scripts/test-commit-conventions.sh | 67 ------------------- 5 files changed, 44 insertions(+), 125 deletions(-) delete mode 100755 scripts/test-commit-conventions.sh diff --git a/.git-hooks/commit-msg b/.git-hooks/commit-msg index 320298f..202f449 100755 --- a/.git-hooks/commit-msg +++ b/.git-hooks/commit-msg @@ -2,37 +2,40 @@ # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 + set -eu -# --subject validates a PR title strictly; file mode is Git's commit-msg hook. -if [ "${1:-}" = "--subject" ]; then - [ "$#" -eq 2 ] || exit 1 - subject=$2 -else - [ "$#" -eq 1 ] || exit 1 - subject=$(sed '/^[[:space:]]*$/d; /^#/d' "$1" | head -n 1) - # Allow Git-generated subjects and temporary rebase commits locally. - case "$subject" in - "Merge "*|"Revert "*|"fixup! "*|"squash! "*|"amend! "*) exit 0 ;; - esac -fi +MSG_FILE="$1" -# Titles and commit subjects must be a single line. -case "$subject" in - *' -'*) echo "Subject must be a single line." >&2; exit 1 ;; +# First non-empty, non-comment line is the commit subject. +SUBJECT=$(grep -vE '^(#|$)' "$MSG_FILE" | head -n1) + +# Skip subjects generated by git. +case "$SUBJECT" in + "Merge "*|"Revert "*|"fixup! "*|"squash! "*|"amend! "*) exit 0 ;; esac -pattern='^(feat|fix|perf|refactor|docs|chore|test|revert)(\([a-z0-9._/-]+\))?!?: [^[:space:]].*$' -if ! printf '%s\n' "$subject" | LC_ALL=C grep -qE "$pattern"; then - cat >&2 <<'EOF' -Expected: (): -Scope is optional. Add ! after the type/scope for a breaking change. -Allowed types: feat, fix, perf, refactor, docs, chore, test, revert +# Conventional-commit format: +# (): +PATTERN='^(feat|fix|perf|refactor|docs|chore|test|revert)(\([a-z0-9._/-]+\))?!?: .+$' + +if ! printf '%s' "$SUBJECT" | grep -qE "$PATTERN"; then + cat >&2 <(): + Examples: - feat(cli): add inventory command - fix: handle missing chassis - feat(api)!: remove deprecated endpoint + feat(cli): add node list command + fix(sdk): handle empty service key + chore: update dependencies + +Allowed types: + feat, fix, perf, refactor, docs, chore, test, revert EOF exit 1 fi diff --git a/.github/workflows/contribution-conventions.yml b/.github/workflows/contribution-conventions.yml index 3027910..33adc79 100644 --- a/.github/workflows/contribution-conventions.yml +++ b/.github/workflows/contribution-conventions.yml @@ -14,13 +14,13 @@ jobs: name: PR title runs-on: ubuntu-latest steps: - - name: Check out code - uses: actions/checkout@v4 - with: - persist-credentials: false - - name: Test subject validation - run: sh scripts/test-commit-conventions.sh - name: Validate PR title env: PR_TITLE: ${{ github.event.pull_request.title }} - run: sh .git-hooks/commit-msg --subject "$PR_TITLE" + shell: bash + run: | + pattern='^(feat|fix|perf|refactor|docs|chore|test|revert)(\([a-z0-9._/-]+\))?!?: [^[:space:]].*$' + if [[ "$PR_TITLE" == *$'\n'* || ! "$PR_TITLE" =~ $pattern ]]; then + echo "::error::Expected PR title: type(scope): description (scope optional; use ! for breaking changes)." + exit 1 + fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e7c52c9..22e7b89 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -120,7 +120,7 @@ Additional labels must be applied to indicate whether the change is a feature, i Local hooks are optional but recommended. They check commit subject formatting and scan staged files for secrets before commit. See the shared conventions -below for the subject format and PR-title validation. +below for the shared conventions and PR-title validation. Install `trufflehog` before enabling the hooks: @@ -183,25 +183,12 @@ final commit subject. Contributors may keep separate review commits; they do not need to squash before review. Preserve all contributors' DCO `Signed-off-by:` trailers in the final squash commit and verify the final message before merging. -### Local validation +### PR title validation -Install the optional commit-format hook from the repository root: - -```bash -make setup-git-hooks -``` - -Run the shared subject-validation tests or check a proposed PR title: - -```bash -make test-commit-conventions -sh .git-hooks/commit-msg --subject "fix(inventory): handle missing chassis" -``` - -The hook allows Git-generated merge/revert subjects and temporary -`fixup!`, `squash!`, and `amend!` commits locally. PR titles must always follow -the format above. The **Contribution conventions / PR title** GitHub Actions check -runs when a PR is opened, edited, or updated. Maintainers should require that -check in branch protection and enable squash merging in repository settings. -The hook checks subject formatting; contributors remain responsible for DCO -sign-offs. +The **Contribution conventions / PR title** GitHub Actions check validates PR +titles when a PR is opened, edited, or updated. Maintainers should require this +check in branch protection and configure squash merging to use the PR title as +the final commit subject. Local commit-format hooks are optional; individual +commit messages do not become target-branch subjects when squash merging. +Contributors must still sign off every commit, and maintainers must preserve +those sign-offs in the final squash commit. diff --git a/Makefile b/Makefile index 7cd6dbe..ec59250 100644 --- a/Makefile +++ b/Makefile @@ -95,7 +95,3 @@ clean: ## Remove local build artifacts .PHONY: help help: ## Show available targets @awk 'BEGIN {FS = ":.*##"} /^[a-zA-Z_-]+:.*##/ {printf "%-12s %s\n", $$1, $$2}' $(MAKEFILE_LIST) - -.PHONY: test-commit-conventions -test-commit-conventions: ## Validate commit subjects and PR titles - sh scripts/test-commit-conventions.sh diff --git a/scripts/test-commit-conventions.sh b/scripts/test-commit-conventions.sh deleted file mode 100755 index 431b4ce..0000000 --- a/scripts/test-commit-conventions.sh +++ /dev/null @@ -1,67 +0,0 @@ -#!/bin/sh - -# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 -set -eu - -cd "$(dirname "$0")/.." -message_file=$(mktemp) -trap 'rm -f "$message_file"' EXIT HUP INT TERM - -accept() { - sh .git-hooks/commit-msg --subject "$1" - printf '# comment\n\n%s\n\nCommit body.\n' "$1" > "$message_file" - sh .git-hooks/commit-msg "$message_file" -} -reject() { - if sh .git-hooks/commit-msg --subject "$1" >/dev/null 2>&1; then - echo "Unexpectedly accepted title: $1" >&2 - exit 1 - fi - printf '%s\n' "$1" > "$message_file" - if sh .git-hooks/commit-msg "$message_file" >/dev/null 2>&1; then - echo "Unexpectedly accepted commit: $1" >&2 - exit 1 - fi -} - -for type in feat fix perf refactor docs chore test revert; do - accept "$type: update behavior" - accept "$type(cli): update behavior" - accept "$type!: change behavior" - accept "$type(api/v2)!: change behavior" -done -reject '' -reject 'add feature' -reject 'Feat: add feature' -reject 'build: update image' -reject 'fix(API): handle missing chassis' -reject 'fix:missing space' -reject 'fix: ' -reject 'fix: ' -reject 'fix: handle missing chassis' - -# Local rebase/merge exceptions must never bypass strict PR-title validation. -for subject in 'Merge branch main' 'Revert "old change"' 'fixup! fix: handle missing chassis' 'squash! feat: add feature' 'amend! docs: update guide'; do - printf '%s\n' "$subject" > "$message_file" - sh .git-hooks/commit-msg "$message_file" - if sh .git-hooks/commit-msg --subject "$subject" >/dev/null 2>&1; then - echo "Unexpectedly accepted Git-generated PR title: $subject" >&2 - exit 1 - fi -done - -: > "$message_file" -if sh .git-hooks/commit-msg "$message_file" >/dev/null 2>&1; then - echo "Unexpectedly accepted empty commit." >&2 - exit 1 -fi -if sh .git-hooks/commit-msg --subject 'fix: first line -fix: second line' >/dev/null 2>&1; then - echo "Unexpectedly accepted multiline title." >&2 - exit 1 -fi - -# Shell metacharacters are data, including in titles supplied through CI env. -accept 'fix(cli): handle $HOME, `commands`, and $(commands) literally' -echo "Contribution convention checks passed."