ci: org-wide defaults - templates, issue routing, hooks, and reusable workflows - #5
ci: org-wide defaults - templates, issue routing, hooks, and reusable workflows#5mc-nv wants to merge 15 commits into
Conversation
- pull_request_template.md + PULL_REQUEST_TEMPLATE/ (internal/external): canonical PR templates inherited by every org repo without local copies - ISSUE_TEMPLATE/config.yml: disables blank issues and routes the new-issue chooser to the server repository - .github/workflows/conventional-pr.yml: reusable workflow validating PR titles against Conventional Commits and labeling the PR with its type TRI-1100
Migrate the add-license pre-commit hook (hardened: .github/ excluded, LICENSE files never rewritten) from developer_tools into this org-wide defaults repository, so hooks, PR/issue templates, and reusable workflows live under one roof with a single tag line. Add this repo's own pre-commit baseline + CI and a self-check caller for the reusable conventional-pr workflow. Consumers reference: - repo: https://github.com/triton-inference-server/.github rev: v1.0.0 hooks: - id: add-license TRI-1100
A trailing "(#N)" squash-merge reference to a different PR in the title or any commit subject now also triggers the cherry-pick label. Ships as v1.3.1. TRI-1100
…is repo The repository ships code (the centralized copyright tool and the reusable conventional-pr workflow) whose file headers assert BSD terms; add the license text itself and run the in-tree add-license hook in this repo's own pre-commit config (templates stay excluded). TRI-1100
Per-repository config-as-code pilot, this repository only: chill profile, auto-review incl. drafts (pilot), path instructions for the org-wide workflows, centralized hook tooling, and inherited templates. Fleet rollout is decided after evaluation; requires the CodeRabbit GitHub App on the organization. No tag changes - this file is not part of any hook or workflow export. TRI-1100
The hook never modifies LICENSE files; instead it now verifies that an NVIDIA copyright line's highest year is the current year and fails with guidance to update it in a deliberate commit. Repos without an NVIDIA copyright line in LICENSE (Apache text, third-party attribution) are not affected. Ships as v1.4.0. TRI-1100
The add-license exclude was too broad: ^\.github/ also skipped workflow files, leaving their copyright headers unmaintained and unchecked. Narrow it to the template paths (frontmatter must stay first) and bump the stale header this repo's own workflow carried. Ships as v1.4.1. TRI-1100
Review feedback (Greptile P1 on backend#127): label creation and mutation use the issues API surface; labeling worked empirically under pull-requests:write alone, but the explicit grant is correct belt-and-braces. Ships as v1.4.2. TRI-1100
Greptile SummaryThis PR establishes the
Confidence Score: 4/5Safe to merge after removing the committed .pyc binary; all functional logic is correct and has been validated live across 24 consumer repos. A compiled Python bytecode file (
Important Files Changed
Sequence DiagramsequenceDiagram
participant PR as Pull Request (any org repo)
participant Caller as consumer caller stub
participant RW as conventional-pr.yml (reusable)
participant GH as GitHub API
PR->>Caller: opened / edited / synchronize
Caller->>RW: workflow_call
RW->>GH: get PR title
RW->>RW: validate title vs TYPES regex
alt title invalid
RW-->>PR: core.setFailed (PR blocked)
else title valid
RW->>GH: paginate listCommits
RW->>RW: derive type set from commits (warnings for non-conforming)
RW->>GH: listLabelsOnIssue
RW->>GH: ensureLabel (create or update color/description)
RW->>GH: removeLabel (stale type labels)
RW->>GH: addLabels (wanted labels)
RW->>RW: cherry-pick detection (trailer / hint / prRef)
opt cherry-pick detected
RW->>GH: ensureLabel + addLabels cherry-pick
end
RW-->>PR: success
end
|
| if (derived.size === 0 && | ||
| !current.some((l) => typeLabels.includes(l.name))) { | ||
| core.setFailed( | ||
| 'No conventional commit type could be determined from the PR ' + | ||
| 'title or its commits, and no type label is assigned.'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Fail-safe condition is unreachable dead code
derived is initialised on line 121 as new Set([match[1]]), and the early return on lines 109–115 guarantees that match[1] is always a valid type token before execution reaches this point. Consequently derived.size is always ≥ 1 here and the derived.size === 0 branch can never fire. The fail-safe described in the PR description ("if no type is derivable … the check fails") is silently a no-op, so any future refactor that removes the new Set([match[1]]) initialiser would produce incorrect labels with no warning.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Intentionally defensive: the explicit requirement was 'if no type is derivable and none is manually assigned, fail'. Today the title gate makes the branch unreachable, but the fail-safe is load-bearing the moment the title requirement is ever relaxed (a direction that has been discussed). Keeping it, with this thread as the record of why.
- add_copyright.py: the double-header sanity check now uses COPYRIGHT_YEAR_PAT so LICENSE forms without '(c)' are covered; the test-section imports carry explicit noqa markers now that flake8's select list is actually applied. - pre-commit workflow: robust modified-files runner - null-delimited paths (spaces survive), --no-run-if-empty (deletion-only PRs), --diff-filter=d (no deleted paths), no undocumented -r flag, explicit contents:read, pre-commit cache keyed on the config hash. - conventional-pr reusable + self-check: contents:read granted explicitly. - add-license exclude also covers the single-file .github/PULL_REQUEST_TEMPLATE.md form. - external PR template: absolute CONTRIBUTING link (relative anchors break in org-inherited templates) and the missing 'chore' type. - flake8 args quoted correctly - the previous flow-scalar form split at commas and silently reduced the select list. Ships as v1.4.3. TRI-1100
Vinya567
left a comment
There was a problem hiding this comment.
LGTM. Greptile P1 on unreachable fail-safe noted; Misha’s rationale for keeping defensive branch makes sense while title gate is required.
|
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 |
What does the PR do?
Establishes this repository as the single org-wide home for shared GitHub assets and tooling:
Inherited defaults (apply automatically to every org repo without local copies):
pull_request_template.md+PULL_REQUEST_TEMPLATE/— canonical PR templates (internal/external), without the manual "Commit Type" checklist (enforced automatically instead).ISSUE_TEMPLATE/config.yml— blank issues disabled; the new-issue chooser redirects to the server repository (server keeps its own localISSUE_TEMPLATE/, which overrides this default and remains the org's issue intake point).Reusable workflow —
.github/workflows/conventional-pr.yml(workflow_call):<type>: <Title>; types: build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test) — hard gate, since the title becomes the squash-merge commit.feat→feature,ci→CI/CD,docs→documentation,fix→fix, …) with org-wide enforced colors and descriptions (drifted labels are corrected; stale/legacy auto-managed labels removed). Non-conforming commit subjects produce warnings; merge commits are ignored.git cherry-pick -xcommit trailer, or a title/branch hint) and applies acherry-picklabel (additive — manual labels are never removed).actions/github-script; no third-party actions. Consumers add a ~10-line caller stub pinned by tag.Centralized pre-commit hook (migrated from developer_tools):
.pre-commit-hooks.yaml+tools/add_copyright.py— theadd-licensehook, hardened:.github/excluded (templates keep YAML frontmatter first) and consumer LICENSE files are never rewritten. Consumers referencerepo: .../.github+ a pinned tag.Also adds this repo's own pre-commit baseline + CI and a self-check caller (
conventional-pr-check.yml, localuses: ./) so PRs here validate themselves with the code they ship.Tags
v1.4.3throughv1.4.3are cut at the corresponding commits of this branch; consumers currently pinv1.4.3(workflow) andv1.4.3+ (hooks). Tags are write-once — future changes ship as a new tag plus rev bumps.Checklist
<commit_type>: <Title>Related PRs:
Where should the reviewer start?
.github/workflows/conventional-pr.yml— the reusable workflow (validation, labeling, cherry-pick detection).pre-commit-hooks.yaml+tools/add_copyright.py— the migrated add-license hook (noteget_license()is read-only)ISSUE_TEMPLATE/config.ymland the template files — the inherited defaultsTest plan:
pre-commit validate-configand all hooks pass on the PR diff (pre-commit check green on this PR).The self-check runs this PR's own workflow version: title validated and the
CI/CDlabel applied with the org color (#bfd4f2) — visible on this PR.add-license verified from a consumer repo with a stale LICENSE year: LICENSE byte-identical after the run; headers inserted with the correct year range.
Live across all 24 consumer PRs (pinned
@v1.4.3): pre-commit and conventional-pr green everywhere; multi-type labeling verified (e.g. backend#127 showsCI/CD+chorederived from its commit history).CI Pipeline ID: N/A — GitHub Actions only (no internal GitLab pipeline for this repository)
Caveats:
Background
TRI-1100 first aligned hooks and templates by placing identical copies in every repo; this PR completes the follow-up decision to consolidate all shared assets under one roof so a single change propagates org-wide. The add-license hook migrates here from developer_tools (its
v0.xtags remain functional for anything still pinned).Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Documentation