ci: validate commit messages instead of PR title - #18699
ci: validate commit messages instead of PR title#18699Christopher Co (christopherco) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The workflow lacks checkout permission and exposes a write-capable token to unlocked dependencies, while its local reproduction commands are incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Replaces PR-title linting with per-commit Conventional Commit validation.
Changes:
- Adds shared commitlint configuration.
- Adds commit-validation workflow and contributor guidance.
- Removes the former PR-title workflow.
File summaries
| File | Description |
|---|---|
CONTRIBUTING.md |
Documents local commit validation. |
commitlint.config.js |
Defines shared commit-message rules. |
.github/workflows/check-pr-title.yml |
Removes PR-title validation. |
.github/workflows/check-commit-messages.yml |
Validates and reports invalid PR commits. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 8
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if: github.repository == 'microsoft/azurelinux' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| pull-requests: write # Needed to post comments on PR |
| # Install commitlint into the workspace root so commitlint.config.js can | ||
| # resolve its `extends`. Pinned to keep the ruleset reproducible. | ||
| - name: Install commitlint | ||
| run: npm install --no-save --no-audit --no-fund @commitlint/cli@21.2.2 @commitlint/config-conventional@21.2.2 |
| const workspace = process.env.GITHUB_WORKSPACE; | ||
| const commitlintBin = path.join(workspace, 'node_modules', '.bin', 'commitlint'); | ||
| const lintMessage = (message) => { | ||
| try { | ||
| execFileSync(commitlintBin, ['--config', 'commitlint.config.js'], { | ||
| input: message, | ||
| cwd: workspace, | ||
| stdio: ['pipe', 'pipe', 'pipe'], | ||
| }); |
| stdio: ['pipe', 'pipe', 'pipe'], | ||
| }); |
| const list = invalid | ||
| .map((c) => `- \`${c.sha}\` <code>${encodeSubject(c.subject)}</code>`) | ||
| .join('\n'); |
| You can reproduce this check locally with the same rules: | ||
|
|
||
| ``` | ||
| npx --yes @commitlint/cli@21 --from origin/4.0 --to HEAD |
| # Lint every commit on your branch that isn't on the target branch: | ||
| npx --yes @commitlint/cli@21 --from origin/4.0 --to HEAD | ||
|
|
||
| # Or check a single message: | ||
| echo "feat(demo): add capability" | npx --yes @commitlint/cli@21 |
| // Validate locally (same rules as CI): | ||
| // npx --yes @commitlint/cli@21 --config commitlint.config.js \ | ||
| // --from origin/4.0 --to HEAD | ||
| // or lint a single message: | ||
| // echo "feat(demo): add capability" | npx --yes @commitlint/cli@21 |
We rebase-merge, so every commit enters the permanent 4.0 history. The old check only validated the PR title, which could hide non-conventional commits behind a conventional title (or fail a well-formed branch behind a descriptive one). Validate the header of every commit with commitlint instead. The rules live in commitlint.config.js so CI and local runs share one source of truth: contributors can reproduce the check with `npm ci` + `npm run commitlint`, and it can later back pre-commit hooks. A github-script step reads the PR commits via the API and lints each against the shared config, allowing optional scopes and breaking-change markers. On failure it comments the offending commits and how to fix them, and removes the comment once all are valid. commitlint and its full dependency tree are pinned via package.json and a committed package-lock.json, installed with `npm ci`, so the ruleset is reproducible and the workflow avoids ad-hoc package installation. The workflow stays on pull_request_target so it can comment on fork PRs. It checks out only the base branch for the config, reads commit metadata through the API, and feeds those messages to commitlint as data -- it never checks out or runs PR code. Commit subjects are HTML-encoded before display, the count is checked against the PR total to fail closed on the 250-commit API cap, and all actions remain SHA-pinned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 886c4e30-30d0-448a-9180-5c4a0ee8018c
3db8dd3 to
942538a
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The workflow will not activate from 4.0 alone and is missing the contents permission required for checkout.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
.github/workflows/check-commit-messages.yml:47
- The job overrides token permissions with only
pull-requests: write, but the checkout step uses the defaultgithub.token;actions/checkoutrequirescontents: readfor reliable repository access. Without it, the job can fail before loading the commitlint config. Grant read-only contents access alongside the existing PR permission.
permissions:
pull-requests: write # Needed to post comments on PR
- Files reviewed: 5/7 changed files
- Comments generated: 1
- Review effort level: Balanced
| on: | ||
| pull_request_target: # zizmor: ignore[dangerous-triggers] |
| @@ -0,0 +1,40 @@ | |||
| // Shared Conventional Commits rules for Azure Linux. | |||
There was a problem hiding this comment.
Not sure if commitlint config and the package.json/package-lock.json (for zizmor check) should really live in root or be in a subdir under .github/
Only requirement is that all 3 files stay together.
We rebase-merge, so every commit enters the permanent 4.0 history. The old check only validated the PR title, which could hide non-conventional commits behind a conventional title (or fail a well-formed branch behind a descriptive one).
Validate the header of every commit with commitlint instead. The rules live in commitlint.config.js so CI and local runs share one source of truth: contributors can reproduce the check with
npx @commitlint/cli, and it can later back pre-commit hooks. A github-script step reads the PR commits via the API and lints each against the shared config, allowing optional scopes and breaking-change markers. On failure it comments the offending commits and how to fix them, and removes the comment once all are valid.The workflow stays on pull_request_target so it can comment on fork PRs. It checks out only the base branch for the config, reads commit metadata through the API, and feeds those messages to commitlint as data -- it never checks out or runs PR code. Commit subjects are HTML-encoded before display, the count is checked against the PR total to fail closed on the 250-commit API cap, and all actions remain SHA-pinned.
Fixes: AB#22437