diff --git a/README.md b/README.md index 0409e3f..b811981 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,30 @@ Note that, typically, you would make this check on a pre-commit hook (for exampl - Changes of type `deprecate:`, `test:`, `ci:`, `style:` and `docs:` must not be breaking. - Commits of type `remove:` must be breaking changes (i.e. `remove!: something`) +### Work in Progress + +Commits of type `WIP` are valid, but they must be squashed before rebasing or merging. + +By default a WIP commit fails the check. That blocks the merge, but it marks the pull +request as failing, and an unfinished branch is an expected state rather than an error. A +check run of its own, concluding `action_required`, blocks the merge without the failure; +only the workflow can create one, so `enforce: false` leaves that reporting to the caller. + +|Name|Type|Description| +|---|---|---| +|`enforce`|input|Whether the action reports the outcome and fails on what it found, WIP commits included (default `true`). With `false` it only produces outputs. Any value other than `true` or `false` is an error| +|`results`|output|The result for every commit, as a JSON array of `{sha, header, level, reason}`, where `level` is `valid`, `wip` or `invalid`| + +With `enforce: false` the action only analyzes: it does not fail on what it found in the +commits and writes no annotations, and the caller reports the outcome from `results`. That +covers invalid commit messages as much as WIP commits: both are in `results`, and both then +need a conclusion from the caller. + +`results` is set on every path, and before the check fails, so it is available whatever the +outcome. If the commit messages cannot be retrieved it is an empty array, and the action +fails on that path even with `enforce: false`, so a caller tells it apart from a pull request +with no findings by the outcome of the step rather than by the output. + ### Semantic Versioning After the action completes, the `SEMVER_LEVEL` environment variable is set according to the highest level of [Semantic Versioning](https://semver.org/spec/v2.0.0.html) change described by the commit messages: diff --git a/action.yml b/action.yml index fd7439e..3150863 100644 --- a/action.yml +++ b/action.yml @@ -1,6 +1,18 @@ name: "Flowing Code Commit Message Guidelines" description: "Ensures that all commit messages are following the Flowing Code Commit Message Guidelines." inputs: + enforce: + description: > + Whether this action reports the outcome and fails on what it found, which includes + failing on Work-in-Progress (WIP) commits. With false it becomes a pure analyzer: + it then only produces outputs, does not fail on what it found in the commits and + writes no annotations, and the caller is expected to report the outcome + (conclusions, annotations) on its own — which is what a caller that reports WIP + commits as a check of its own asks for. + Any value other than true or false is an error, so that a misspelling is not read + as the default. + required: false + default: "true" token: description: > The token used to read the commits of a pull request. It defaults to the token of @@ -10,6 +22,12 @@ inputs: commits anonymously. required: false default: ${{ github.token }} +outputs: + results: + description: > + The result for every commit, as a JSON array of objects with a "sha", a + "header", a "level" ("valid", "wip" or "invalid") and, for invalid commits, + a "reason". runs: using: node20 main: dist/main/index.js diff --git a/dist/main/index.js b/dist/main/index.js index 31121ce..bcd4e35 100644 --- a/dist/main/index.js +++ b/dist/main/index.js @@ -62646,7 +62646,10 @@ const MAJOR_COMMIT_TYPES = [ "remove", ]; const isWIP = (message) => { - return message.startsWith("WIP:"); + // The type is parsed rather than matched as a prefix: "WIP(scope):" and "WIP!:" are + // accepted by validateCommitMessage, and were reported as valid commits. + const match = message.match(/^(\w+)(\(\S+?\))?(!?): /); + return match !== null && match[1] === "WIP"; }; const validateCommitMessage = (message) => { let [header] = message.split('\n'); @@ -62781,7 +62784,8 @@ const extractCommits = (context, token) => __awaiter(void 0, void 0, void 0, fun // For "push" events, commits can be found in the "context.payload.commits". const pushCommits = Array.isArray(lodash_get_default()(context, "payload.commits")); if (pushCommits) { - return context.payload.commits; + core.info(`ℹ️ Read ${context.payload.commits.length} commit(s) from the push payload.`); + return context.payload.commits.map((commit) => ({ message: commit.message, sha: commit.id })); } // For PRs, we need to get a list of commits via the GH API: const prCommitsUrl = lodash_get_default()(context, "payload.pull_request.commits_url"); @@ -62790,9 +62794,12 @@ const extractCommits = (context, token) => __awaiter(void 0, void 0, void 0, fun core.warning(`⚠️ The commits of the pull request are being read anonymously, which GitHub rate limits per IP address and which cannot read a private repository. The token input is empty: unless that is deliberate, it is a mistake in the configuration.`); } const items = yield readCommits(prCommitsUrl, token); - core.info(`ℹ️ Read ${items.length} commit(s).`); - return items.map((item) => item.commit); + core.info(`ℹ️ Read ${items.length} commit(s) from the pull request.`); + return items.map((item) => ({ message: item.commit.message, sha: item.sha })); } + // Neither a push nor a pull request: there is nothing to read, and saying which event + // it was keeps this apart from a push or a pull request that carries no commits. + core.info(`ℹ️ No commits to check: the "${context.eventName}" event has neither a push payload nor a pull request.`); return []; }); /* harmony default export */ const src_extractCommits = (extractCommits); @@ -62811,27 +62818,48 @@ const { context } = __nccwpck_require__(5438); const main_core = __nccwpck_require__(2186); +function setOutputs(semverLevel, results) { + main_core.exportVariable('SEMVER_LEVEL', semverLevel.toString()); + main_core.setOutput('results', JSON.stringify(results)); +} function run() { return main_awaiter(this, void 0, void 0, function* () { main_core.info(`ℹ️ Checking if commit messages are following the Flowing Code Commit Message Guidelines...`); + // action.yml supplies the default whenever the action is called as one, so the input + // is absent only when the bundle runs outside Actions, and the fallback is for that + // alone. A value that is present but empty was written by the caller — an unset + // workflow input interpolated into it, say — and is an error like any other value + // that is neither true nor false, rather than silently the default. + const input = process.env.INPUT_ENFORCE; + const value = input === undefined ? 'true' : input.trim(); + if (value !== 'true' && value !== 'false') { + setOutputs(0, []); + main_core.setFailed(`🚫 The enforce input must be true or false, not "${value}".`); + return; + } + /** Whether this action reports the outcome and fails on what it found. When false, + it only produces outputs, and the caller is expected to report the outcome. */ + const enforce = value === 'true'; let extractedCommits; try { extractedCommits = yield src_extractCommits(context, main_core.getInput('token')); } catch (error) { + // Reporting is left to the caller only for the outcome of the analysis. // Not being able to analyse anything is a failure of the action itself. - // SEMVER_LEVEL is exported nonetheless, so that a later step reading it - // does not read an empty value. - main_core.exportVariable('SEMVER_LEVEL', '0'); + setOutputs(0, []); main_core.setFailed(`🚫 The commit messages could not be checked: ${error instanceof Error ? error.message : error}`); return; } let semverLevel = 0; let hasErrors = false; let hasWIP = false; + const results = []; main_core.startGroup("Commit messages:"); for (let i = 0; i < extractedCommits.length; i++) { let commit = extractedCommits[i]; + const header = commit.message.split('\n')[0]; + const sha = commit.sha; let errmsg = validateCommitMessage(commit.message); if (errmsg === null) { const commitSemverLevel = getSemverLevel(commit.message); @@ -62839,24 +62867,40 @@ function run() { semverLevel = commitSemverLevel; if (isWIP(commit.message)) { hasWIP = true; + results.push({ sha, header, level: 'wip' }); main_core.info(`🚧 ${commit.message}`); } else { + results.push({ sha, header, level: 'valid' }); main_core.info(`✅ ${commit.message}`); } } else { - main_core.info(`🚩 ${commit.message} : ${errmsg}`); + results.push({ sha, header, level: 'invalid', reason: errmsg }); hasErrors = true; + // When this action reports, core.error creates an annotation on the check + // run, so the offending commit is visible on the pull request itself. + const digest = sha ? `${sha.substring(0, 7)} ` : ''; + const line = `🚩 ${digest}${header} : ${errmsg}`; + if (enforce) + main_core.error(line); + else + main_core.info(line); } } main_core.endGroup(); - main_core.exportVariable('SEMVER_LEVEL', semverLevel.toString()); + setOutputs(semverLevel, results); + if (!enforce) + return; if (hasErrors) { main_core.setFailed(`🚫 According to the Flowing Code Commit Message Guidelines, some of the commit messages are not valid.`); } else if (hasWIP) { - main_core.setFailed(`🚧 Work-in-Progress (WIP) commits found.`); + // A WIP commit must not be merged, and a step cannot both block the merge and + // avoid the red X: that needs a check run of its own, which only a caller can + // create. So the action keeps failing, and a caller that reports WIP for itself + // asks for enforce: false rather than being handed a green check by default. + main_core.setFailed(`🚧 Work-in-Progress (WIP) commits found. They must be squashed before rebasing or merging.`); } else if (extractedCommits.length === 0) { main_core.info(`No commits to check, skipping...`); diff --git a/src/__tests__/isValidCommitMessage.test.ts b/src/__tests__/isValidCommitMessage.test.ts index 8d03509..a319134 100644 --- a/src/__tests__/isValidCommitMessage.test.ts +++ b/src/__tests__/isValidCommitMessage.test.ts @@ -66,6 +66,8 @@ test("should be able to correctly parse the semver level", () => { test("should be able to correctly detect WIP commits", () => { expect(isWIP("WIP: foo")).toBe(true); + expect(isWIP("WIP(scope): foo")).toBe(true); + expect(isWIP("WIP!: foo")).toBe(true); expect(isWIP("wip: foo")).toBe(false); expect(isWIP("WIP")).toBe(false); expect(isWIP("fix: foo")).toBe(false); diff --git a/src/extractCommits.ts b/src/extractCommits.ts index a3d6fad..4319c1a 100644 --- a/src/extractCommits.ts +++ b/src/extractCommits.ts @@ -5,6 +5,7 @@ import got from "got"; type Commit = { message: string; + sha?: string; }; /** The URL of the next page, taken from the Link header of a response. */ @@ -42,7 +43,8 @@ const extractCommits = async (context, token?: string): Promise => { // For "push" events, commits can be found in the "context.payload.commits". const pushCommits = Array.isArray(get(context, "payload.commits")); if (pushCommits) { - return context.payload.commits; + core.info(`ℹ️ Read ${context.payload.commits.length} commit(s) from the push payload.`); + return context.payload.commits.map((commit) => ({message: commit.message, sha: commit.id})); } // For PRs, we need to get a list of commits via the GH API: @@ -55,10 +57,13 @@ const extractCommits = async (context, token?: string): Promise => { } const items = await readCommits(prCommitsUrl, token); - core.info(`ℹ️ Read ${items.length} commit(s).`); - return items.map((item) => item.commit); + core.info(`ℹ️ Read ${items.length} commit(s) from the pull request.`); + return items.map((item) => ({message: item.commit.message, sha: item.sha})); } + // Neither a push nor a pull request: there is nothing to read, and saying which event + // it was keeps this apart from a push or a pull request that carries no commits. + core.info(`ℹ️ No commits to check: the "${context.eventName}" event has neither a push payload nor a pull request.`); return []; }; diff --git a/src/isValidCommitMesage.ts b/src/isValidCommitMesage.ts index 11cccb9..587b139 100644 --- a/src/isValidCommitMesage.ts +++ b/src/isValidCommitMesage.ts @@ -41,8 +41,11 @@ const MAJOR_COMMIT_TYPES = [ export type SemverLevel = 0 | 1 | 2 | 3; -export const isWIP = (message): string | null => { - return message.startsWith("WIP:"); +export const isWIP = (message): boolean => { + // The type is parsed rather than matched as a prefix: "WIP(scope):" and "WIP!:" are + // accepted by validateCommitMessage, and were reported as valid commits. + const match = message.match(/^(\w+)(\(\S+?\))?(!?): /); + return match !== null && match[1] === "WIP"; } export const validateCommitMessage = (message): string | null => { diff --git a/src/main.ts b/src/main.ts index eab29df..6e38b47 100644 --- a/src/main.ts +++ b/src/main.ts @@ -4,31 +4,59 @@ const core = require("@actions/core"); import {validateCommitMessage, isWIP, getSemverLevel, SemverLevel} from "./isValidCommitMesage"; import extractCommits from "./extractCommits"; +type Result = {sha?: string, header: string, level: string, reason?: string}; + +function setOutputs(semverLevel: SemverLevel, results: Result[]) { + core.exportVariable('SEMVER_LEVEL', semverLevel.toString()); + core.setOutput('results', JSON.stringify(results)); +} + async function run() { core.info( `ℹ️ Checking if commit messages are following the Flowing Code Commit Message Guidelines...` ); + // action.yml supplies the default whenever the action is called as one, so the input + // is absent only when the bundle runs outside Actions, and the fallback is for that + // alone. A value that is present but empty was written by the caller — an unset + // workflow input interpolated into it, say — and is an error like any other value + // that is neither true nor false, rather than silently the default. + const input = process.env.INPUT_ENFORCE; + const value = input === undefined ? 'true' : input.trim(); + if (value !== 'true' && value !== 'false') { + setOutputs(0, []); + core.setFailed( + `🚫 The enforce input must be true or false, not "${value}".` + ); + return; + } + + /** Whether this action reports the outcome and fails on what it found. When false, + it only produces outputs, and the caller is expected to report the outcome. */ + const enforce = value === 'true'; + let extractedCommits; try { extractedCommits = await extractCommits(context, core.getInput('token')); } catch (error) { + // Reporting is left to the caller only for the outcome of the analysis. // Not being able to analyse anything is a failure of the action itself. - // SEMVER_LEVEL is exported nonetheless, so that a later step reading it - // does not read an empty value. - core.exportVariable('SEMVER_LEVEL', '0'); + setOutputs(0, []); core.setFailed( `🚫 The commit messages could not be checked: ${error instanceof Error ? error.message : error}` ); return; } - + let semverLevel : SemverLevel = 0; let hasErrors = false; let hasWIP = false; + const results : Result[] = []; core.startGroup("Commit messages:"); for (let i = 0; i < extractedCommits.length; i++) { let commit = extractedCommits[i]; + const header = commit.message.split('\n')[0]; + const sha = commit.sha; let errmsg = validateCommitMessage(commit.message); if (errmsg === null) { @@ -36,25 +64,40 @@ async function run() { if (commitSemverLevel>semverLevel) semverLevel=commitSemverLevel; if (isWIP(commit.message)) { hasWIP = true; + results.push({sha, header, level: 'wip'}); core.info(`🚧 ${commit.message}`); } else { + results.push({sha, header, level: 'valid'}); core.info(`✅ ${commit.message}`); } } else { - core.info(`🚩 ${commit.message} : ${errmsg}`); + results.push({sha, header, level: 'invalid', reason: errmsg}); hasErrors = true; + // When this action reports, core.error creates an annotation on the check + // run, so the offending commit is visible on the pull request itself. + const digest = sha ? `${sha.substring(0, 7)} ` : ''; + const line = `🚩 ${digest}${header} : ${errmsg}`; + if (enforce) core.error(line); else core.info(line); } } core.endGroup(); - core.exportVariable('SEMVER_LEVEL', semverLevel.toString()); + setOutputs(semverLevel, results); + if (!enforce) return; + if (hasErrors) { core.setFailed( `🚫 According to the Flowing Code Commit Message Guidelines, some of the commit messages are not valid.` ); } else if (hasWIP) { - core.setFailed(`🚧 Work-in-Progress (WIP) commits found.`); + // A WIP commit must not be merged, and a step cannot both block the merge and + // avoid the red X: that needs a check run of its own, which only a caller can + // create. So the action keeps failing, and a caller that reports WIP for itself + // asks for enforce: false rather than being handed a green check by default. + core.setFailed( + `🚧 Work-in-Progress (WIP) commits found. They must be squashed before rebasing or merging.` + ); } else if (extractedCommits.length === 0) { core.info(`No commits to check, skipping...`); } else {