From e38a1611212276dfa72ff8fd2d75394d773aaf61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliv=C3=A9r=20Kecskem=C3=A9ty?= Date: Sat, 24 Oct 2026 00:03:23 +0200 Subject: [PATCH] Set services apart from the job header and steps --- docs/workflows.md | 2 +- scripts/workflow-style.mjs | 21 ++++++++++++++++++--- scripts/workflow-style.test.mjs | 13 +++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/docs/workflows.md b/docs/workflows.md index 18c3dbf..998f69e 100644 --- a/docs/workflows.md +++ b/docs/workflows.md @@ -38,7 +38,7 @@ Every GitHub Actions workflow and composite action in the studio's repositories ## Spacing and quoting -- Exactly one empty line separates top-level keys, jobs and steps; there is none right before `steps`. +- Exactly one empty line separates top-level keys, jobs and steps; there is none right before `steps`. A job with `services` puts them right before `steps`, with one empty line before and after them. - Strings are quoted only where YAML requires it, with double quotes. ## Composite actions diff --git a/scripts/workflow-style.mjs b/scripts/workflow-style.mjs index 0e995df..2cc2bbc 100644 --- a/scripts/workflow-style.mjs +++ b/scripts/workflow-style.mjs @@ -162,7 +162,7 @@ function checkValues(root, report) { } } -function checkSteps(steps, kinds, report, { allowShell }) { +function checkSteps(steps, kinds, report, { allowShell, afterServices }) { const items = steps ? steps.children.filter((entry) => entry.isItem) : []; items.forEach((item, index) => { if (index > 0 && blanksBetween(kinds, items[index - 1].endLine, item.line) !== 1) report(item.line, "one empty line between steps"); @@ -174,7 +174,22 @@ function checkSteps(steps, kinds, report, { allowShell }) { const shell = child(item, "shell"); if (shell && !allowShell) report(shell.line, "no per-step shell"); }); - if (steps && blankAbove(kinds, steps.line)) report(steps.line, "no empty line before steps"); + if (!steps) return; + if (afterServices) { + if (blanksBetween(kinds, afterServices.endLine, steps.line) !== 1) report(steps.line, "one empty line between services and steps"); + } else if (blankAbove(kinds, steps.line)) report(steps.line, "no empty line before steps"); +} + +// Returns the services node when it sits right before steps, so the steps check knows to expect an empty line. +function checkServices(job, kinds, report) { + const services = child(job, "services"); + if (!services) return null; + const jobKeys = keys(job); + const index = jobKeys.indexOf(services); + if (blanksBetween(kinds, index > 0 ? jobKeys[index - 1].endLine : job.line, services.line) !== 1) report(services.line, "one empty line before services"); + if (jobKeys[index + 1]?.key === "steps") return services; + report(services.line, "services must come right before steps"); + return null; } function checkTopLevel(root, kinds, report, { workflow }) { @@ -214,7 +229,7 @@ function checkWorkflowRules(root, kinds, report) { if (index > 0 && blanksBetween(kinds, jobs[index - 1].endLine, job.line) !== 1) report(job.line, "one empty line between jobs"); if (!child(job, "name")) report(job.line, `job "${job.key}" has no name`); if (!child(job, "timeout-minutes") && !child(job, "uses")) report(job.line, `job "${job.key}" has no timeout-minutes`); - checkSteps(child(job, "steps"), kinds, report, { allowShell: false }); + checkSteps(child(job, "steps"), kinds, report, { allowShell: false, afterServices: checkServices(job, kinds, report) }); }); } diff --git a/scripts/workflow-style.test.mjs b/scripts/workflow-style.test.mjs index e9cd7f8..edfcb92 100644 --- a/scripts/workflow-style.test.mjs +++ b/scripts/workflow-style.test.mjs @@ -220,6 +220,19 @@ test("no empty line before steps", () => { assert.deepEqual(check(text), [{ line: 23, message: "no empty line before steps" }]); }); +test("services sit between empty lines right before steps", () => { + const services = [" services:", " postgres:", " image: postgres:18"]; + assert.deepEqual(check(doc(header, job.slice(0, 4), "", services, "", " steps:", step)), []); + assert.deepEqual(check(doc(header, job.slice(0, 4), "", " # Databases for the tests.", services, "", " # The suite.", " steps:", step)), []); + assert.deepEqual(check(doc(header, job.slice(0, 4), services, " steps:", step)), [ + { line: 22, message: "one empty line before services" }, + { line: 25, message: "one empty line between services and steps" }, + ]); + assert.deepEqual(check(doc(header, job.slice(0, 3), "", services, " timeout-minutes: 10", " steps:", step)), [ + { line: 22, message: "services must come right before steps" }, + ]); +}); + test("spacing between jobs and top-level keys", () => { const text = doc(header.slice(0, 4), header.slice(5), job, step, " lint:", " name: Lint", " runs-on: ubuntu-latest", " timeout-minutes: 10", " steps:", step); assert.deepEqual(check(text), [