Skip to content

ci: align pre-commit hooks and GitHub templates - #80

Closed
mc-nv wants to merge 18 commits into
mainfrom
mchornyi/TRI-1100/github-align-hooks-and-templates
Closed

ci: align pre-commit hooks and GitHub templates#80
mc-nv wants to merge 18 commits into
mainfrom
mchornyi/TRI-1100/github-align-hooks-and-templates

Conversation

@mc-nv

@mc-nv mc-nv commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

What does the PR do?

Aligns this repository with the org-wide setup consolidated in triton-inference-server/.github:

  • .pre-commit-config.yaml: shared baseline hooks, conventional-commit message validation (commit-msg stage), and the centralized add-license hook from the org .github repository (rev: v1.4.3 — excludes .github/ templates, never rewrites LICENSE files).
  • Pre-commit CI runs only on files modified by the PR.
  • Caller stub for the org-wide reusable conventional-pr workflow (@v1.4.3): validates the PR title against Conventional Commits (hard gate — it becomes the squash-merge commit), derives one human-readable label per distinct type found in the title and all conforming commit subjects (e.g. ci:CI/CD, feat:feature, fix:fix), enforces org-wide label colors/descriptions, detects cherry-picks, and fails if no type is derivable and no type label is assigned.
  • No per-repo templates: PR templates and issue routing are inherited from the org-wide defaults in triton-inference-server/.github (issues route to the server repository).

Repo-specific: adds pyproject.toml with the shared codespell/isort settings.

Depends on triton-inference-server/.github#5 (pinned tags current: v1.4.3 — already exist, CI is green).

Pros / Cons

Pros

  • Single home (org .github repo) for hooks, templates, issue routing, and the PR-title workflow — one change propagates everywhere.
  • Repos carry only a config file and two small workflow stubs; no template copies to drift.
  • Commit/PR title format enforced both locally (commit-msg hook) and in CI (reusable workflow), with automatic type labels.
  • License hooks never modify LICENSE files.

Cons / risks

  • Version-pinned dependency on the org .github repository (tags are write-once; changes ship as a new tag + rev bump).
  • Template inheritance requires the org .github repository to remain public.

Related Issues / PRs

  • Resolves: TRI-1100

Related PRs:

Test plan

  • pre-commit validate-config passes; pre-commit run --files <PR diff> passes locally with the centralized hooks pinned to the .github branch SHA.
  • After .github#5 merges and v1.4.3 exists: the conventional-pr check validates this PR's own title and applies the ci label.
  • No LICENSE file content is changed by this PR.

Caveats

  • None beyond the merge-order note above.

Checklist

  • PR title follows <commit_type>: <Title> (conventional commit)
  • I ran pre-commit locally on all files changed by this PR and it passes
  • Copyright header is correct on all changed files
  • External contributors: I have read the Contribution guidelines and signed the Contributor License Agreement

Adopt the shared pre-commit baseline: two-line SPDX header, conventional
commit message validation (commit-msg stage), and the centralized
add-license / add-spdx-license hooks from developer_tools v0.2.0.
Run pre-commit CI only on files modified by the PR, and roll out the
standard issue templates and the simplified single PR template.

TRI-1100
@mc-nv mc-nv self-assigned this Jul 18, 2026
This was referenced Jul 18, 2026
Legal's Copyright / License Header Guidance specifies the SPDX form
without a comma after the year.

TRI-1100
@mc-nv
mc-nv marked this pull request as ready for review July 18, 2026 02:05
Human-readable type labels with enforced descriptions and colors.

TRI-1100
@github-actions github-actions Bot added CI/CD Continuous integration and workflow changes (ci: PRs) and removed ci labels Jul 20, 2026
@github-actions github-actions Bot added the chore Maintenance work, no production code change (chore: PRs) label Jul 20, 2026
mc-nv added 4 commits July 21, 2026 08:16
The add-license hook now fails when the LICENSE copyright year is
stale.

TRI-1100
Workflow files are license-processed again (only templates excluded);
refresh the stale copyright year this repo's pre-commit workflow
carried.

TRI-1100
Review feedback on #80: main excluded "\.patch$" per hook; the
baseline alignment dropped it. Vendored patches are byte- and
whitespace-exact - trailing-whitespace, end-of-file-fixer, or the
formatters would corrupt any touched patch. Restore it as a single
top-level exclude covering every hook, including the centralized
add-license.

TRI-1100
Grant issues:write to the labeling job (review feedback).

TRI-1100
@mc-nv
mc-nv requested a review from pskiran1 July 22, 2026 15:12
@Vinya567

Copy link
Copy Markdown

@greptileai

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR aligns the triton-inference-server/third_party repository with the org-wide CI/configuration standard defined in triton-inference-server/.github. It replaces per-repo hook definitions and templates with thin caller stubs that delegate to versioned reusable workflows and centralized hooks, and adds a pyproject.toml for shared codespell/isort settings.

  • Adds .github/workflows/conventional-pr.yml — a caller stub for the org-wide conventional-pr reusable workflow (@v1.4.3), using a fork-safe pull_request / pull_request_target routing pattern and job-scoped minimal permissions.
  • Rewrites .github/workflows/pre-commit.yml to run pre-commit directly (with null-delimited file list for space-safe paths, deletion filter, and a cache step keyed on the config hash) instead of the pre-commit/action action.
  • Overhauls .pre-commit-config.yaml to move the global exclude pattern to the top level, add conventional-pre-commit for commit-msg validation, wire codespell to pyproject.toml, and migrate the add-license hook from developer_tools to the org .github repository at v1.4.3.

Confidence Score: 5/5

Safe to merge — all changes are CI/configuration only, no application code is affected, and the fork-safe routing pattern for pull_request_target is correctly implemented with job-scoped minimal permissions.

The PR introduces only workflow stubs, a pre-commit config overhaul, and a new pyproject.toml. The pull_request_target routing logic is correct and well-commented, hook exclusions are properly centralized, and the reusable workflow is pinned by tag with write-once semantics documented. No application logic is touched.

No files require special attention, though conventional-pr.yml and pre-commit.yml carry minor hardening opportunities noted in the review comments.

Important Files Changed

Filename Overview
.github/workflows/conventional-pr.yml New caller stub for org-wide conventional-pr reusable workflow; correctly separates same-repo (pull_request) and fork (pull_request_target) events with job-scoped minimal permissions, but lacks a workflow-level permissions block for defense-in-depth.
.github/workflows/pre-commit.yml Pre-commit workflow rewritten to use null-delimited xargs for space-safe paths, deletion filter, and a cache step; replaces pre-commit/action with direct pip install but does not pin the pre-commit version.
.pre-commit-config.yaml Global exclude moved to top level, conventional-pre-commit added for commit-msg stage, codespell wired to pyproject.toml, add-license migrated to org .github repo at v1.4.3; changes look correct.
pyproject.toml New file with shared codespell and isort settings; configuration looks consistent with the hook arguments in .pre-commit-config.yaml.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant GH as GitHub Events
    participant CPR as conventional-pr.yml
    participant RW as Reusable Workflow<br/>(org .github@v1.4.3)
    participant PC as pre-commit.yml
    participant Cache as actions/cache
    participant Hooks as Pre-commit Hooks<br/>(.pre-commit-config.yaml)

    Dev->>GH: Push commit (commit-msg hook runs locally)
    Note over Dev: conventional-pre-commit<br/>validates commit message

    Dev->>GH: Open / update PR
    GH->>CPR: pull_request (same-repo) OR pull_request_target (fork)
    CPR->>CPR: Route by event + head.repo check
    CPR->>RW: "uses conventional-pr.yml@v1.4.3"
    RW->>GH: Validate PR title (Conventional Commits)
    RW->>GH: Derive and apply type label
    RW->>GH: Detect cherry-picks

    GH->>PC: pull_request event
    PC->>Cache: Restore ~/.cache/pre-commit
    PC->>PC: "git diff --name-only -z --diff-filter=d HEAD^1 HEAD"
    PC->>Hooks: pre-commit run --files modified-files
    Hooks->>Hooks: isort, black, flake8, clang-format, codespell, add-license
Loading

Reviews (2): Last reviewed commit: "ci: harden CI workflows and configs per ..." | Re-trigger Greptile

Comment thread .pre-commit-config.yaml Outdated
Comment on lines 91 to 92
rev: v1.4.2
hooks:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Version tag mismatch with PR description

The PR description explicitly states v1.4.1 at least three times ("pinned tags current: v1.4.1 — already exist, CI is green", "caller stub...@v1.4.1", "centralized add-license hook...rev: v1.4.1"), but both this file and conventional-pr.yml pin to v1.4.2. If v1.4.2 is not yet published in the org .github repository, pre-commit install will fail to resolve the hook and the add-license step will error at runtime for every contributor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed - documentation drift: the descriptions have been refreshed to the current pinned tag v1.4.3 (they lagged behind the tag revisions; the tag exists and CI is green fleet-wide).

Comment thread .pre-commit-config.yaml
- conventional-pr stub: dual pull_request/pull_request_target triggers
  so fork PRs from external contributors get labeled too (the reusable
  workflow never checks out PR code); explicit contents:read; pinned
  v1.4.3.
- pre-commit workflow: robust modified-files runner (null-delimited
  paths, deletion-only PRs handled, deleted paths filtered, no
  undocumented -r flag, cache keyed on config hash).
- flake8 args quoted correctly (the flow-scalar form split at commas
  and silently reduced the select list).
- hooks pinned to .github v1.4.3.

TRI-1100
@mc-nv

mc-nv commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Closing: the team is moving away from centralized org-level configuration in favor of per-repository self-contained setups (see triton-inference-server/server#8897 for the first decentralized implementation). Branch retained for reference. TRI-1100

@mc-nv mc-nv closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance work, no production code change (chore: PRs) CI/CD Continuous integration and workflow changes (ci: PRs)

Development

Successfully merging this pull request may close these issues.

3 participants