From 58f002f950109ef1ffe11f4d18039a930f176ab4 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sun, 1 Mar 2026 10:14:27 -0500 Subject: [PATCH] ci/github-script/lint-commits: error when conventional commit format is used E.g. https://redirect.github.com/NixOS/nixpkgs/pull/495442 --- ci/github-script/get-pr-commit-details.js | 18 ++++++- ci/github-script/lint-commits.js | 63 +++++++++++++++++++++-- 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/ci/github-script/get-pr-commit-details.js b/ci/github-script/get-pr-commit-details.js index c9349fa7e055..fcccfeacd75e 100644 --- a/ci/github-script/get-pr-commit-details.js +++ b/ci/github-script/get-pr-commit-details.js @@ -23,6 +23,11 @@ async function runGit({ args, repoPath, core, quiet }) { } /** + * Gets the SHA, subject and changed files for each commit in the given PR. + * + * Don't use GitHub API at all: the "list commits on PR" endpoint has a limit + * of 250 commits and doesn't return the changed files. + * * @param {{ * core: import('@actions/core'), * pr: Awaited["rest"]["pulls"]["get"]>>["data"] @@ -32,6 +37,8 @@ async function runGit({ args, repoPath, core, quiet }) { * @returns {Promise<{ * subject: string, * sha: string, + * changedPaths: string[], + * changedPathSegments: Set, * }[]>} */ async function getCommitDetailsForPR({ core, pr, repoPath }) { @@ -63,9 +70,10 @@ async function getCommitDetailsForPR({ core, pr, repoPath }) { return Promise.all( shas.map(async (sha) => { + // Subject first, then a blank line, then filenames. const result = ( await runGit({ - args: ['log', '--format=%s', '--numstat', '-1', sha], + args: ['log', '--format=%s', '--name-only', '-1', sha], repoPath, core, quiet: true, @@ -74,9 +82,17 @@ async function getCommitDetailsForPR({ core, pr, repoPath }) { const subject = result[0] + const changedPaths = result.slice(2, -1) + + const changedPathSegments = new Set( + changedPaths.flatMap((path) => path.split('/')), + ) + return { sha, subject, + changedPaths, + changedPathSegments, } }), ) diff --git a/ci/github-script/lint-commits.js b/ci/github-script/lint-commits.js index bba4cb9ec99a..4ef8b6f6ab04 100644 --- a/ci/github-script/lint-commits.js +++ b/ci/github-script/lint-commits.js @@ -46,17 +46,60 @@ async function checkCommitMessages({ github, context, core, repoPath }) { return } - const commits = await getCommitDetailsForPR({ - core, - pr, - repoPath, - }) + const commits = await getCommitDetailsForPR({ core, pr, repoPath }) const failures = new Set() + const conventionalCommitTypes = [ + 'build', + 'chore', + 'ci', + 'doc', + 'docs', + 'feat', + 'feature', + 'fix', + 'perf', + 'refactor', + 'style', + 'test', + ] + + /** + * @param {string[]} types e.g. ["fix", "feat"] + * @param {string?} sha commit hash + */ + function makeConventionalCommitRegex(types, sha = null) { + core.info( + `${ + sha + ? `Conventional commit types for ${sha?.slice(0, 16)}` + : 'Default conventional commit types' + }: ${JSON.stringify(types)}`, + ) + + return new RegExp(`^(${types.join('|')})!?(\\(.*\\))?!?:`) + } + + // Optimize for the common case that we don't have path segments with the + // same name as a conventional commit type. + const fullConventionalCommitRegex = makeConventionalCommitRegex( + conventionalCommitTypes, + ) + for (const commit of commits) { const logMsgStart = `Commit ${commit.sha}'s message's subject ("${commit.subject}")` + // If we have a commit `perf: ...`, and we touch a file containing the path + // segment "perf", we don't want to flag this. + const filteredTypes = conventionalCommitTypes.filter( + (type) => !commit.changedPathSegments.has(type), + ) + const conventionalCommitRegex = + filteredTypes.length === conventionalCommitTypes.length + ? fullConventionalCommitRegex + : makeConventionalCommitRegex(filteredTypes, commit.sha) + if (!commit.subject.includes(': ')) { core.error( `${logMsgStart} was detected as not meeting our guidelines because ` + @@ -84,6 +127,16 @@ async function checkCommitMessages({ github, context, core, repoPath }) { failures.add(commit.sha) } + if (conventionalCommitRegex.test(commit.subject)) { + core.error( + `${logMsgStart} was detected as not meeting our guidelines because ` + + 'it seems to use conventional commit (conventionalcommits.org) ' + + 'formatting. Nixpkgs has its own, different, commit message ' + + 'formatting standards.', + ) + failures.add(commit.sha) + } + if (!failures.has(commit.sha)) { core.info(`${logMsgStart} passed our automated checks!`) }