mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-04 02:14:01 +00:00
feat(tooling): one changelog file per pull request
`CONTRIBUTING.md` asks every runtime change to add an entry at the top of `CHANGELOG.md`, so two pull requests in flight write into the same few lines and git has nothing to merge on. Against `main` at `81eb443`, `git merge-tree` over the 85 open pull requests GitHub reports as conflicting that aren't mine puts `CHANGELOG.md` in the conflict set of 41 — more than any other path — and for 6 it is the only one. `docs/CURRENT-STATE.md`, the other file rule 3 names, conflicts for 23, and merges cleanly for 23 of the 41: edits land in the section for the area you touched, so they only collide when the areas do. A pull request now adds `changelog.d/<pull request>-<slug>.md` instead. Distinct filenames never conflict, and the pull request number makes them distinct for free. `npm run changelog` splices the directory into `CHANGELOG.md` and empties it; `npm run changelog:check` reports the batch and changes nothing. Fragment text is copied through unchanged apart from trailing whitespace. `src/annotations/drawTool.test.mjs` reads `CHANGELOG.md` as prose and slices a fixed window after a heading, so an assembler that reflowed an entry could move text across that window. Fragments are deliberately outside `scripts/format-scope.json` for the same reason. The assembler refuses a batch before writing anything when a filename is not `<pull request>-<slug>.md` or two fragments claim one pull request number, and a unit test runs that check over the real `changelog.d/`, so a fragment the release could not assemble fails `npm test` in the pull request that adds it rather than at release time. Refs #781.
This commit is contained in:
1 parent
81eb44340d
commit
114b0eeaba
7 files changed
+359
-3
No files matched your search
+1
-1
@@ -149,7 +149,7 @@ ownership and adoption process.
|
||||
|
||||
1. Branch off `main`.
|
||||
2. Keep `npm run build`, `npm test`, and `npm run test:track` green and avoid new console errors, plus the [feature gate](#feature-regression-gates) for the area you touched.
|
||||
3. If you change runtime behavior, update `docs/CURRENT-STATE.md` and `CHANGELOG.md` in the same PR.
|
||||
3. If you change runtime behavior, update `docs/CURRENT-STATE.md` in the same PR, and add your changelog entry as `changelog.d/<pr-number>-<slug>.md` instead of editing `CHANGELOG.md` — see [changelog.d/README.md](changelog.d/README.md). Distinct filenames don't conflict; a release assembles the directory into `CHANGELOG.md`. A documentation claim that needs a regression test should assert against `docs/CURRENT-STATE.md` or `README.md`, which a PR still edits directly.
|
||||
4. If you add or change a data source, update [DATA_SOURCES.md](DATA_SOURCES.md) with its license and attribution. **Don't add data you don't have the right to redistribute** — fetch it at runtime instead.
|
||||
5. Describe what you changed and how you verified it (screenshots welcome for anything visual).
|
||||
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
# Changelog fragments
|
||||
|
||||
One file per entry. A pull request adds a file here instead of editing
|
||||
`CHANGELOG.md`, and a release concatenates them into it.
|
||||
|
||||
```
|
||||
changelog.d/<pull request number>-<slug>.md
|
||||
```
|
||||
|
||||
`723-atc-decision-layer.md`, for example. The slug is lowercase, words separated
|
||||
by `-`. Two pull requests never pick the same filename, because the pull request
|
||||
number is already unique — which is the point: new `CHANGELOG.md` entries go at
|
||||
the top of the file, so two pull requests in flight write into the same few lines
|
||||
and git has nothing to merge on.
|
||||
|
||||
Write the file exactly as the entry should read in `CHANGELOG.md`: a `-` bullet,
|
||||
wrapped the way the surrounding entries are, crediting yourself and the pull
|
||||
request.
|
||||
|
||||
```markdown
|
||||
- Report which upstream declined a Street Traffic road load. The layer row now
|
||||
reads `Overpass rate-limited` instead of a general "Road data temporarily
|
||||
unavailable" (yourname, #665).
|
||||
```
|
||||
|
||||
The text is spliced in unchanged apart from trailing whitespace, so what you
|
||||
write here is what ships. Fragments are deliberately outside
|
||||
`scripts/format-scope.json`, so `npm run format` never reflows one.
|
||||
|
||||
At release time:
|
||||
|
||||
```
|
||||
npm run changelog:check # list what would move, change nothing
|
||||
npm run changelog # move every fragment into CHANGELOG.md
|
||||
```
|
||||
|
||||
The assembler refuses a batch rather than guessing: a filename that is not
|
||||
`<pull request>-<slug>.md`, or two fragments claiming one pull request number,
|
||||
stops the run before anything is written and names the files.
|
||||
|
||||
A documentation claim that needs a regression test — the way
|
||||
`src/annotations/drawTool.test.mjs` asserts against `CHANGELOG.md` — should
|
||||
assert against `docs/CURRENT-STATE.md` or `README.md` instead. A pull request
|
||||
still edits those directly, so the text is in the file while the change is under
|
||||
review; a fragment does not reach `CHANGELOG.md` until release.
|
||||
@@ -57,7 +57,8 @@ with a reason; usefulness can also be `decline` or `discuss`.
|
||||
- Inspect removed tests, skipped tests, relaxed assertions, snapshot updates, mocks,
|
||||
and test-runner changes. A green result from weakened tests is not acceptance.
|
||||
- Check applicable documentation updates: runtime changes need `CURRENT-STATE.md`
|
||||
and `CHANGELOG.md`; source changes need `DATA_SOURCES.md`.
|
||||
and a `changelog.d/<pr-number>-<slug>.md` fragment rather than an edit to
|
||||
`CHANGELOG.md`; source changes need `DATA_SOURCES.md`.
|
||||
|
||||
### 3. Security and supply chain
|
||||
|
||||
@@ -173,6 +174,14 @@ Before merging:
|
||||
a stale merge. Verify the merged result matches the validated candidate tree
|
||||
and record the resulting commit and PR URL. Stop and report any mismatch.
|
||||
|
||||
Pending release notes live in `changelog.d/` as one file per pull request, so an
|
||||
integration never has to resolve the top of `CHANGELOG.md`. When cutting a release,
|
||||
run `npm run changelog:check` to see the batch and the order it would take, then
|
||||
`npm run changelog` to splice it into `CHANGELOG.md` and empty the directory. The
|
||||
assembler refuses a malformed filename or two fragments claiming one pull request
|
||||
number before it writes anything, and copies the text through unchanged, so the
|
||||
contributor's own wording and credit are what ship.
|
||||
|
||||
Repository rules enforce only their configured requirements; this document and
|
||||
the skill do not configure branch protection or CI. Maintainers should require
|
||||
CI and review on `main` and dismiss stale approvals when changes are pushed.
|
||||
|
||||
@@ -37,6 +37,8 @@
|
||||
"format:check": "node scripts/format.mjs --check",
|
||||
"layer-token:next": "node scripts/next-layer-state-token.mjs",
|
||||
"layer-token:check": "node scripts/check-layer-state-tokens.mjs",
|
||||
"changelog": "node scripts/assemble-changelog.mjs --write",
|
||||
"changelog:check": "node scripts/assemble-changelog.mjs --check",
|
||||
"dev": "vite",
|
||||
"dev:secure": "./scripts/dev-secure.sh",
|
||||
"opensky:import": "./scripts/opensky-import-client.sh",
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
import { readdir, readFile, writeFile, rm } from 'node:fs/promises';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath, pathToFileURL } from 'node:url';
|
||||
|
||||
/**
|
||||
* Release-time assembly of changelog fragments.
|
||||
*
|
||||
* A pull request adds `changelog.d/<pull request>-<slug>.md` instead of editing
|
||||
* `CHANGELOG.md`. Distinct filenames never conflict, so two pull requests in
|
||||
* flight no longer write into the same few lines at the top of one file. A
|
||||
* release splices the fragments in and empties the directory.
|
||||
*
|
||||
* Fragment text is copied through unchanged apart from trailing whitespace:
|
||||
* `src/annotations/drawTool.test.mjs` reads `CHANGELOG.md` as prose and slices a
|
||||
* fixed window after a heading, so reflowing an entry here could move text
|
||||
* across that window.
|
||||
*/
|
||||
export const FRAGMENT_DIR = 'changelog.d';
|
||||
const FRAGMENT_NAME = /^(\d+)-[a-z0-9]+(?:-[a-z0-9]+)*\.md$/;
|
||||
const HEADING = '# Changelog\n';
|
||||
|
||||
/** Fragment file names a release may consume, highest pull request first. */
|
||||
export function orderFragmentNames(names) {
|
||||
const claimed = new Map();
|
||||
for (const name of names) {
|
||||
if (name === 'README.md') continue;
|
||||
const match = FRAGMENT_NAME.exec(name);
|
||||
if (!match) {
|
||||
throw new Error(
|
||||
`Unexpected file in ${FRAGMENT_DIR}/: ${name}` +
|
||||
' — expected <pull request>-<slug>.md with a lowercase slug',
|
||||
);
|
||||
}
|
||||
const number = Number(match[1]);
|
||||
const prior = claimed.get(number);
|
||||
if (prior) {
|
||||
throw new Error(
|
||||
`Two fragments claim pull request ${number}: ${prior} and ${name}`,
|
||||
);
|
||||
}
|
||||
claimed.set(number, name);
|
||||
}
|
||||
return [...claimed.entries()]
|
||||
.sort(([a], [b]) => b - a)
|
||||
.map(([, name]) => name);
|
||||
}
|
||||
|
||||
/** Splice fragment bodies above the existing entries, newest batch on top. */
|
||||
export function assembleChangelog(changelog, fragments) {
|
||||
if (!fragments.length) return changelog;
|
||||
if (!changelog.startsWith(HEADING)) {
|
||||
throw new Error('CHANGELOG.md must start with "# Changelog"');
|
||||
}
|
||||
const entries = changelog.slice(HEADING.length).replace(/^\n+/, '');
|
||||
const batch = fragments.map((text) => text.replace(/\s+$/, '')).join('\n\n');
|
||||
return `${HEADING}\n${batch}\n\n${entries}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Report or apply the pending fragments in `<root>/changelog.d`.
|
||||
*
|
||||
* The batch is validated in full before anything is written, so a malformed
|
||||
* filename or a duplicate pull request number leaves the directory untouched.
|
||||
*
|
||||
* @param {string} root repository root
|
||||
* @param {'--check'|'--write'} mode `--check` reports and changes nothing
|
||||
* @returns {Promise<{ consumed: string[], changelog: string }>}
|
||||
*/
|
||||
export async function assembleFragments(root, mode) {
|
||||
if (!['--check', '--write'].includes(mode)) {
|
||||
throw new Error(
|
||||
'Usage: node scripts/assemble-changelog.mjs --check|--write',
|
||||
);
|
||||
}
|
||||
const directory = path.join(root, FRAGMENT_DIR);
|
||||
const changelogFile = path.join(root, 'CHANGELOG.md');
|
||||
const consumed = orderFragmentNames(await readdir(directory));
|
||||
const bodies = [];
|
||||
for (const name of consumed) {
|
||||
bodies.push(await readFile(path.join(directory, name), 'utf8'));
|
||||
}
|
||||
const changelog = assembleChangelog(
|
||||
await readFile(changelogFile, 'utf8'),
|
||||
bodies,
|
||||
);
|
||||
if (mode === '--write' && consumed.length) {
|
||||
await writeFile(changelogFile, changelog, 'utf8');
|
||||
for (const name of consumed) await rm(path.join(directory, name));
|
||||
}
|
||||
return { consumed, changelog };
|
||||
}
|
||||
|
||||
const invoked = process.argv[1]
|
||||
? pathToFileURL(path.resolve(process.argv[1])).href
|
||||
: '';
|
||||
if (import.meta.url === invoked) {
|
||||
try {
|
||||
if (process.argv.length !== 3) {
|
||||
throw new Error(
|
||||
'Usage: node scripts/assemble-changelog.mjs --check|--write',
|
||||
);
|
||||
}
|
||||
const mode = process.argv[2];
|
||||
const { consumed } = await assembleFragments(
|
||||
fileURLToPath(new URL('../', import.meta.url)),
|
||||
mode,
|
||||
);
|
||||
if (!consumed.length) {
|
||||
console.log(`No fragments in ${FRAGMENT_DIR}/.`);
|
||||
} else {
|
||||
console.log(
|
||||
`${mode === '--write' ? 'Assembled' : 'Would assemble'} ${consumed.length} fragment(s), in order:` +
|
||||
`\n ${consumed.join('\n ')}`,
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
console.error(error.message);
|
||||
process.exitCode = 1;
|
||||
}
|
||||
}
|
||||
@@ -844,5 +844,7 @@
|
||||
"src/ui/imageryBoxTool.test.mjs",
|
||||
"src/ui/recentImagery.test.mjs",
|
||||
"src/layers/recentImagery/testDoubles.mjs",
|
||||
"scripts/qa-recent-imagery.mjs"
|
||||
"scripts/qa-recent-imagery.mjs",
|
||||
"scripts/assemble-changelog.mjs",
|
||||
"src/tooling/changelogFragments.test.mjs"
|
||||
]
|
||||
@@ -0,0 +1,178 @@
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {
|
||||
mkdtemp,
|
||||
mkdir,
|
||||
writeFile,
|
||||
readFile,
|
||||
readdir,
|
||||
rm,
|
||||
} from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
import {
|
||||
FRAGMENT_DIR,
|
||||
assembleChangelog,
|
||||
assembleFragments,
|
||||
orderFragmentNames,
|
||||
} from '../../scripts/assemble-changelog.mjs';
|
||||
|
||||
const EXISTING =
|
||||
'# Changelog\n\n- An entry that was already released (someone, #1).\n';
|
||||
|
||||
const repositoryRoot = fileURLToPath(new URL('../..', import.meta.url));
|
||||
|
||||
async function fixture(t, fragments = {}) {
|
||||
const root = await mkdtemp(path.join(tmpdir(), 'gev-changelog-'));
|
||||
t.after(() => rm(root, { recursive: true, force: true }));
|
||||
await mkdir(path.join(root, FRAGMENT_DIR));
|
||||
await writeFile(
|
||||
path.join(root, FRAGMENT_DIR, 'README.md'),
|
||||
'# How to add one\n',
|
||||
);
|
||||
await writeFile(path.join(root, 'CHANGELOG.md'), EXISTING);
|
||||
for (const [name, body] of Object.entries(fragments)) {
|
||||
await writeFile(path.join(root, FRAGMENT_DIR, name), body);
|
||||
}
|
||||
return root;
|
||||
}
|
||||
|
||||
test('the batch reads newest pull request first', () => {
|
||||
assert.deepEqual(orderFragmentNames(['7-b.md', '112-c.md', '19-a.md']), [
|
||||
'112-c.md',
|
||||
'19-a.md',
|
||||
'7-b.md',
|
||||
]);
|
||||
});
|
||||
|
||||
test('the directory README is documentation, never an entry', () => {
|
||||
assert.deepEqual(orderFragmentNames(['README.md', '5-a.md']), ['5-a.md']);
|
||||
});
|
||||
|
||||
test('two fragments claiming one pull request fail loudly, not silently', () => {
|
||||
assert.throws(
|
||||
() => orderFragmentNames(['5-first.md', '5-second.md']),
|
||||
/Two fragments claim pull request 5/,
|
||||
);
|
||||
});
|
||||
|
||||
test('a name that is not <pull request>-<slug>.md is refused', () => {
|
||||
for (const name of ['notes.md', '5_underscore.md', '5-Caps.md', '5-a.txt']) {
|
||||
assert.throws(() => orderFragmentNames([name]), /Unexpected file/, name);
|
||||
}
|
||||
});
|
||||
|
||||
test('fragment text is spliced in unchanged, above the existing entries', () => {
|
||||
// Wrapped, indented and punctuated the way a real entry is: the continuation
|
||||
// indent and the blank line inside the entry are the parts a reflow would eat.
|
||||
const entry = [
|
||||
'- A new thing, with `code` and a — dash. The second line is indented the',
|
||||
' way the surrounding entries wrap, and it keeps its two spaces.',
|
||||
'',
|
||||
' A second paragraph, because some entries have one (someone, #9).',
|
||||
].join('\n');
|
||||
const result = assembleChangelog(EXISTING, [`${entry}\n\n\n`]);
|
||||
assert.equal(
|
||||
result,
|
||||
`# Changelog\n\n${entry}\n\n- An entry that was already released (someone, #1).\n`,
|
||||
);
|
||||
// The entry survives byte for byte: drawTool.test.mjs reads this file as
|
||||
// prose and slices a fixed window, so reflowing would move text across it.
|
||||
assert.ok(result.includes(entry));
|
||||
});
|
||||
|
||||
test('a CHANGELOG without the heading is refused rather than prepended to', () => {
|
||||
assert.throws(
|
||||
() => assembleChangelog('- orphan entry\n', ['- new\n']),
|
||||
/must start with "# Changelog"/,
|
||||
);
|
||||
});
|
||||
|
||||
test('assembling consumes the fragments and leaves the README', async (t) => {
|
||||
const root = await fixture(t, {
|
||||
'20-second.md': '- Second (someone, #20).',
|
||||
'100-first.md': '- First (someone, #100).',
|
||||
});
|
||||
const { consumed } = await assembleFragments(root, '--write');
|
||||
assert.deepEqual(consumed, ['100-first.md', '20-second.md']);
|
||||
const changelog = await readFile(path.join(root, 'CHANGELOG.md'), 'utf8');
|
||||
assert.match(
|
||||
changelog,
|
||||
/# Changelog\n\n- First \(someone, #100\)\.\n\n- Second \(someone, #20\)\.\n\n- An entry that was already released/,
|
||||
);
|
||||
assert.deepEqual(await readdir(path.join(root, FRAGMENT_DIR)), ['README.md']);
|
||||
});
|
||||
|
||||
test('a rejected batch leaves the directory and the file untouched', async (t) => {
|
||||
const root = await fixture(t, {
|
||||
'8-first.md': '- First (someone, #8).',
|
||||
'8-again.md': '- Again (someone, #8).',
|
||||
});
|
||||
await assert.rejects(
|
||||
assembleFragments(root, '--write'),
|
||||
/Two fragments claim pull request 8/,
|
||||
);
|
||||
assert.equal(
|
||||
await readFile(path.join(root, 'CHANGELOG.md'), 'utf8'),
|
||||
EXISTING,
|
||||
);
|
||||
assert.deepEqual((await readdir(path.join(root, FRAGMENT_DIR))).sort(), [
|
||||
'8-again.md',
|
||||
'8-first.md',
|
||||
'README.md',
|
||||
]);
|
||||
});
|
||||
|
||||
test('check mode changes nothing on disk', async (t) => {
|
||||
const root = await fixture(t, { '3-a.md': '- Something (someone, #3).' });
|
||||
const { consumed } = await assembleFragments(root, '--check');
|
||||
assert.deepEqual(consumed, ['3-a.md']);
|
||||
assert.equal(
|
||||
await readFile(path.join(root, 'CHANGELOG.md'), 'utf8'),
|
||||
EXISTING,
|
||||
);
|
||||
assert.deepEqual((await readdir(path.join(root, FRAGMENT_DIR))).sort(), [
|
||||
'3-a.md',
|
||||
'README.md',
|
||||
]);
|
||||
});
|
||||
|
||||
test('an empty directory is a no-op, so a release can always run it', async (t) => {
|
||||
const root = await fixture(t);
|
||||
const { consumed } = await assembleFragments(root, '--write');
|
||||
assert.deepEqual(consumed, []);
|
||||
assert.equal(
|
||||
await readFile(path.join(root, 'CHANGELOG.md'), 'utf8'),
|
||||
EXISTING,
|
||||
);
|
||||
});
|
||||
|
||||
test('a mode that is neither check nor write is refused', async (t) => {
|
||||
const root = await fixture(t);
|
||||
await assert.rejects(assembleFragments(root, '--force'), /--check\|--write/);
|
||||
});
|
||||
|
||||
// The two below run against the repository itself, so a fragment that the
|
||||
// release could not assemble fails `npm test` in the pull request that adds it
|
||||
// rather than at release time, when its author has moved on.
|
||||
|
||||
test('every pending fragment in the repository can be assembled', async () => {
|
||||
const names = await readdir(path.join(repositoryRoot, FRAGMENT_DIR));
|
||||
assert.ok(
|
||||
names.includes('README.md'),
|
||||
`${FRAGMENT_DIR}/README.md documents the convention and keeps the directory tracked`,
|
||||
);
|
||||
// Throws on a name that is not <pull request>-<slug>.md, and on two fragments
|
||||
// claiming one pull request number. Every other file is an entry.
|
||||
assert.equal(orderFragmentNames(names).length, names.length - 1);
|
||||
});
|
||||
|
||||
test('CHANGELOG.md still carries the heading a release splices under', async () => {
|
||||
const changelog = await readFile(
|
||||
path.join(repositoryRoot, 'CHANGELOG.md'),
|
||||
'utf8',
|
||||
);
|
||||
assert.ok(changelog.startsWith('# Changelog\n'));
|
||||
});
|
||||
Reference in new issue
Block a user