Skip to content
Open
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
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ jobs:
- name: Type-check
run: yarn typecheck

- name: Type-check bin
run: yarn typecheck:bin

- name: Check type-check programs use live source
run: yarn tsx bin/check-typecheck-sources.ts

Expand Down
145 changes: 0 additions & 145 deletions bin/check-changelog.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { describe, expect, it } from "vitest";
import {
changelogMessage,
formatProblemsMessage,
impactLineProblems,
missingPackageChangelogs,
needsChangelog,
sectionProblems,
} from "./check-changelog.js";

const packagePrefixes = ["packages/format", "packages/pointers"];
Expand Down Expand Up @@ -231,145 +228,3 @@ describe("changelogMessage", () => {
expect(message).toContain("packages/pointers/CHANGELOG.md");
});
});

describe("impactLineProblems", () => {
const entry = (producers: string, consumers: string): string =>
[
"## Unreleased",
"",
"### Changed",
"",
"- A summary of the change ([#1]).",
" - Schemas: **ethdebug/format/pointer**",
` - Producers: ${producers}`,
` - Consumers: ${consumers}`,
"",
].join("\n");

it("accepts the no change needed. prefix", () => {
expect(
impactLineProblems(
entry("no change needed.", "no change needed. One short reason."),
),
).toEqual([]);
});

it("accepts the optional: prefix", () => {
expect(
impactLineProblems(
entry("optional: a producer may emit it.", "optional: may read it."),
),
).toEqual([]);
});

it("accepts the required: prefix", () => {
expect(
impactLineProblems(
entry("required: `minimum` forbids it.", "required: must read it."),
),
).toEqual([]);
});

it("accepts a prefix on the line below a bare label", () => {
const text = [
"- A summary.",
" - Producers:",
" optional: a producer may emit it.",
"",
].join("\n");
expect(impactLineProblems(text)).toEqual([]);
});

it("reports a bare imperative with its line number", () => {
const problems = impactLineProblems(
entry("emit the new field.", "no change needed."),
);
expect(problems).toHaveLength(1);
expect(problems[0]).toContain("line 7:");
expect(problems[0]).toContain('"Producers:"');
expect(problems[0]).toContain('"no change needed."');
expect(problems[0]).toContain('"optional:"');
expect(problems[0]).toContain('"required:"');
});

it("reports each offending sub-item", () => {
const problems = impactLineProblems(
entry("emit the new field.", "Required: read the new field."),
);
expect(problems).toHaveLength(2);
expect(problems[0]).toContain("line 7:");
expect(problems[1]).toContain("line 8:");
expect(problems[1]).toContain('"Consumers:"');
});

it("reports a prefix that runs into the text after it", () => {
expect(
impactLineProblems(entry("optional:emit it.", "no change needed.")),
).toHaveLength(1);
});

it("reports a bare label with no prefix below it", () => {
const text = ["- A summary.", " - Consumers:", " read it.", ""].join(
"\n",
);
expect(impactLineProblems(text)).toEqual([
expect.stringContaining("line 2:"),
]);
});

it("ignores the intro bullets that describe the sub-items", () => {
const text = [
"# Changelog",
"",
"- `Schemas:` the schema(s) the change touches.",
"- `Producers:` what the change means for an emitter of data (a",
" compiler such as solc or bugc).",
"- `Consumers:` what the change means for a reader.",
"",
"Each `Producers:` and `Consumers:` sub-item starts with a prefix:",
"",
"- `no change needed.` Nothing changes.",
"",
].join("\n");
expect(impactLineProblems(text)).toEqual([]);
});

it("accepts a file with no entries", () => {
expect(impactLineProblems("# Changelog\n\n## Unreleased\n")).toEqual([]);
expect(impactLineProblems("")).toEqual([]);
});
});

describe("formatProblemsMessage", () => {
it("is empty when there are no problems", () => {
expect(formatProblemsMessage([])).toBe("");
});

it("names the file and lists each problem", () => {
const message = formatProblemsMessage(["line 7: first", "line 9: second"]);
expect(message).toContain("CHANGELOG.md");
expect(message).toContain(" line 7: first");
expect(message).toContain(" line 9: second");
});
});

describe("sectionProblems", () => {
it("accepts Added and Changed sections", () => {
const text = "## Unreleased\n\n### Added\n\n### Changed\n";
expect(sectionProblems(text)).toEqual([]);
});

it("rejects any other section with its line number", () => {
const text = "## Unreleased\n\n### Breaking\n";
const problems = sectionProblems(text);
expect(problems).toHaveLength(1);
expect(problems[0]).toContain("line 3");
expect(problems[0]).toContain('"### Added"');
expect(problems[0]).toContain('"### Changed"');
});

it("ignores version headings and deeper headings", () => {
expect(sectionProblems("# Changelog\n\n## 0.1.0-1\n")).toEqual([]);
expect(sectionProblems("#### Breaking\n")).toEqual([]);
});
});
97 changes: 16 additions & 81 deletions bin/check-changelog.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { execFileSync } from "node:child_process";
import { readFileSync } from "node:fs";
import { join, relative } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { readWorkspaces } from "./publish-tagged.js";
import {
formatProblemsMessage,
impactLineProblems,
sectionProblems,
} from "./release/changelog.js";
import * as git from "./release/git.js";
import { readWorkspaces } from "./release/workspaces.js";

const defaultBase = "origin/main";

Expand Down Expand Up @@ -89,89 +94,19 @@
return lines.join("\n");
}

const impactPrefixes = ["no change needed.", "optional:", "required:"];

// a real sub-item is indented two spaces and has a bare label; the intro
// bullets that describe the sub-items start at column 0 with a code span
const impactLabel = /^ {2}- (Producers|Consumers):(.*)$/;

function startsWithImpactPrefix(text: string): boolean {
return impactPrefixes.some(
(prefix) =>
text.startsWith(prefix) &&
(text.length === prefix.length || /\s/.test(text[prefix.length])),
);
}

export function impactLineProblems(text: string): string[] {
const lines = text.split("\n");
const allowed = impactPrefixes.map((prefix) => `"${prefix}"`).join(", ");
return lines.flatMap((line, index) => {
const match = impactLabel.exec(line);
if (!match) {
return [];
}
const [, label, rest] = match;
// the text starts on the label line after one space, or, when the
// label stands alone, on the continuation line below it
const conforms =
rest.trim().length > 0
? rest.startsWith(" ") && startsWithImpactPrefix(rest.slice(1))
: startsWithImpactPrefix((lines[index + 1] ?? "").trimStart());
return conforms
? []
: [`line ${index + 1}: "${label}:" must start with one of: ${allowed}`];
});
}

const sectionNames = ["Added", "Changed"];

// the prefixes carry the obligations, so a section only says whether a
// change adds something new or alters something that exists
export function sectionProblems(text: string): string[] {
const allowed = sectionNames.map((name) => `"### ${name}"`).join(", ");
return text.split("\n").flatMap((line, index) => {
if (!line.startsWith("### ")) {
return [];
}
return sectionNames.includes(line.slice(4).trim())
? []
: [`line ${index + 1}: section heading must be one of: ${allowed}`];
});
}

export function formatProblemsMessage(problems: string[]): string {
if (problems.length === 0) {
return "";
}
return [
"CHANGELOG.md does not follow the entry format:",
...problems.map((problem) => ` ${problem}`),
].join("\n");
}

function resolvesToCommit(root: string, ref: string): boolean {
try {
execFileSync(
"git",
["rev-parse", "--verify", "--quiet", `${ref}^{commit}`],
{
cwd: root,
stdio: "ignore",
},
);
return true;
} catch {
return false;
}
return (
git.status(root, [
"rev-parse",
"--verify",
"--quiet",
`${ref}^{commit}`,
]) === 0
);
}

function changedPaths(root: string, base: string): string[] {
const stdout = execFileSync(
"git",
["diff", "--name-only", `${base}...HEAD`],
{ cwd: root, encoding: "utf8" },
);
const stdout = git.run(root, ["diff", "--name-only", `${base}...HEAD`]);
return stdout
.split("\n")
.map((line) => line.trim())
Expand All @@ -188,7 +123,7 @@
const base = argv[0] ?? defaultBase;
const root = fileURLToPath(new URL("..", import.meta.url));
if (!resolvesToCommit(root, base)) {
console.error(

Check warning on line 126 in bin/check-changelog.ts

View workflow job for this annotation

GitHub Actions / lint-and-format

Unexpected console statement
`check-changelog: cannot resolve the base ref "${base}"; fetch it` +
" first, or name one that exists.",
);
Expand All @@ -207,10 +142,10 @@
formatProblemsMessage(problems),
].filter((message) => message.length > 0);
if (messages.length > 0) {
console.error(messages.join("\n\n"));

Check warning on line 145 in bin/check-changelog.ts

View workflow job for this annotation

GitHub Actions / lint-and-format

Unexpected console statement
return 1;
}
console.log("changelog: ok");

Check warning on line 148 in bin/check-changelog.ts

View workflow job for this annotation

GitHub Actions / lint-and-format

Unexpected console statement
return 0;
}

Expand Down
4 changes: 2 additions & 2 deletions bin/check-tarballs.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { fileURLToPath } from "node:url";
import { checkPackList, packList } from "./packlist.js";
import { readWorkspaces } from "./publish-tagged.js";
import { checkPackList, packList } from "./release/packlist.js";
import { readWorkspaces } from "./release/workspaces.js";

const root = fileURLToPath(new URL("..", import.meta.url));

Expand All @@ -12,12 +12,12 @@
const bad = checkPackList(packList(workspace.dir));
if (bad.length > 0) {
failed = true;
console.error(`${workspace.name}: disallowed files in tarball:`);

Check warning on line 15 in bin/check-tarballs.ts

View workflow job for this annotation

GitHub Actions / lint-and-format

Unexpected console statement
for (const path of bad) {
console.error(` ${path}`);

Check warning on line 17 in bin/check-tarballs.ts

View workflow job for this annotation

GitHub Actions / lint-and-format

Unexpected console statement
}
} else {
console.log(`${workspace.name}: ok`);

Check warning on line 20 in bin/check-tarballs.ts

View workflow job for this annotation

GitHub Actions / lint-and-format

Unexpected console statement
}
}
process.exit(failed ? 1 : 0);
2 changes: 1 addition & 1 deletion bin/check-typecheck-sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { existsSync } from "node:fs";
import { join } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { readWorkspaces } from "./publish-tagged.js";
import { readWorkspaces } from "./release/workspaces.js";

const built = [/packages\/[^/]+\/dist\//, /node_modules\/@ethdebug\//];

Expand Down Expand Up @@ -36,7 +36,7 @@
const bad = builtFiles(listFiles(root, workspace.dir));
if (bad.length > 0) {
failed = true;
console.error(`${workspace.name}: type-check reads built declarations:`);

Check warning on line 39 in bin/check-typecheck-sources.ts

View workflow job for this annotation

GitHub Actions / lint-and-format

Unexpected console statement
for (const path of bad) {
console.error(` ${path}`);
}
Expand Down
Loading
Loading