Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
- Plain strings go unquoted; patterns such as globs and cron expressions, and strings YAML requires quoted, use double quotes.

## Composite actions
Expand Down
21 changes: 18 additions & 3 deletions scripts/workflow-style.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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 }) {
Expand Down Expand Up @@ -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) });
});
}

Expand Down
13 changes: 13 additions & 0 deletions scripts/workflow-style.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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), [
Expand Down
Loading