mirror of
https://github.com/clusterio/clusterio.git
synced 2026-11-11 15:04:52 +09:00
Require an impact for each known compat break
Entries now explain why end users are not or only minimally affected, and the docs say which version to key by and point to the breaking changes decision. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9bd6effd40
commit
3c2ee3e372
5 files changed
+44
-25
No files matched your search
+11
-4
@@ -255,20 +255,27 @@ If there are no changelog entries to add, remove the code block and start the ch
|
||||
|
||||
The version-compat job in CI runs the local controller with the latest published host, and the published controller with the local host.
|
||||
It fails if a link message changed its src, dst, data or response schema since that release.
|
||||
If the break is intended, add an entry for the published version to [scripts/version-compat-breaks.json](../scripts/version-compat-breaks.json) in the same pull request:
|
||||
If the break is intended, add an entry to [scripts/version-compat-breaks.json](../scripts/version-compat-breaks.json) in the same pull request.
|
||||
Breaking changes are only accepted if the next release is intended to include them, see our decision on [breaking changes](/docs/decisions.md#breaking-changes).
|
||||
The entry is keyed by the published version directly before the breaking change was introduced, which is the latest release at the time.
|
||||
Every entry needs the pull request number, a reason saying what breaks, and an impact saying why end users are not or only minimally affected.
|
||||
|
||||
```json
|
||||
{
|
||||
"2.0.3": {
|
||||
"messages": {
|
||||
"request FactorioVersionsRequest": { "pr": 1129, "reason": "Why it breaks and what stops working." }
|
||||
"request FactorioVersionsRequest": {
|
||||
"pr": 1129,
|
||||
"reason": "What breaks between the two versions.",
|
||||
"impact": "Why end users are not or only minimally affected."
|
||||
}
|
||||
},
|
||||
"connect": { "pr": 1234, "reason": "Why a host cannot connect at all." }
|
||||
"connect": { "pr": 1234, "reason": "Why a host cannot connect at all.", "impact": "..." }
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Listed messages are reported as warnings with the reason instead of failing the job.
|
||||
Listed messages are reported as warnings with the reason and impact instead of failing the job.
|
||||
Only add `connect` when the published and local versions cannot connect at all, it skips starting the cluster.
|
||||
Listing a message that did not change fails the job.
|
||||
Entries only apply while their version is the latest release, after the next release they are ignored and can be removed.
|
||||
|
||||
@@ -7,9 +7,9 @@
|
||||
* {
|
||||
* "2.0.3": {
|
||||
* "messages": {
|
||||
* "request FactorioVersionsRequest": { "pr": 1129, "reason": "..." }
|
||||
* "request FactorioVersionsRequest": { "pr": 1129, "reason": "...", "impact": "..." }
|
||||
* },
|
||||
* "connect": { "pr": 1234, "reason": "..." }
|
||||
* "connect": { "pr": 1234, "reason": "...", "impact": "..." }
|
||||
* }
|
||||
* }
|
||||
*
|
||||
@@ -30,10 +30,12 @@ function validateBreak(value, where) {
|
||||
if (!Number.isInteger(value.pr) || value.pr <= 0) {
|
||||
throw new Error(`${where} must have a pr number`);
|
||||
}
|
||||
if (typeof value.reason !== "string" || !value.reason.trim()) {
|
||||
throw new Error(`${where} must have a reason`);
|
||||
for (const key of ["reason", "impact"]) {
|
||||
if (typeof value[key] !== "string" || !value[key].trim()) {
|
||||
throw new Error(`${where} is missing ${key}`);
|
||||
}
|
||||
}
|
||||
const unknown = Object.keys(value).filter(key => !["pr", "reason"].includes(key));
|
||||
const unknown = Object.keys(value).filter(key => !["pr", "reason", "impact"].includes(key));
|
||||
if (unknown.length) {
|
||||
throw new Error(`${where} has unknown properties ${unknown.join(", ")}`);
|
||||
}
|
||||
|
||||
@@ -3,11 +3,13 @@
|
||||
"messages": {
|
||||
"request FactorioVersionsRequest": {
|
||||
"pr": 1129,
|
||||
"reason": "Hosts send it while starting an instance, so a newer host cannot start instances on a 2.0.3 controller."
|
||||
"reason": "Hosts send it while starting an instance, which a 2.0.3 controller rejects so the start fails.",
|
||||
"impact": "The host stays connected and the instance goes back to stopped with the error shown, starting it works again once the controller is updated."
|
||||
},
|
||||
"request LatestReleasesRequest": {
|
||||
"pr": 1129,
|
||||
"reason": "Hosts send it while starting an instance that targets a release channel such as stable, which a 2.0.3 controller rejects."
|
||||
"reason": "Hosts send it while starting an instance that targets a release channel such as stable.",
|
||||
"impact": "Hosts send FactorioVersionsRequest first, so a 2.0.3 controller rejects the start before this is sent, starting it works again once the controller is updated."
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -247,8 +247,8 @@ async function checkMessageCompat(publishedVersion, knownBreaks) {
|
||||
const { known, unknown, stale } = matchBreaks(knownBreaks, changed.map(({ name }) => name));
|
||||
for (const { name, fields, localSpec, publishedSpec } of changed) {
|
||||
if (known.includes(name)) {
|
||||
const { pr, reason } = knownBreaks.get(name);
|
||||
annotate(`Link ${name} is a known break since ${publishedVersion} from #${pr}: ${reason}`);
|
||||
const { pr, reason, impact } = knownBreaks.get(name);
|
||||
annotate(`Link ${name} is a known break since ${publishedVersion} from #${pr}: ${reason} Impact: ${impact}`);
|
||||
printChange(console.log, name, fields, localSpec, publishedSpec, publishedVersion);
|
||||
} else {
|
||||
printChange(console.error, name, fields, localSpec, publishedSpec, publishedVersion);
|
||||
@@ -308,8 +308,10 @@ async function main() {
|
||||
}
|
||||
await checkMessageCompat(publishedLib.version, knownBreaks.messages);
|
||||
if (knownBreaks.connect) {
|
||||
const { pr, reason } = knownBreaks.connect;
|
||||
annotate(`Skipping cluster test, known break since ${publishedLib.version} from #${pr}: ${reason}`);
|
||||
const { pr, reason, impact } = knownBreaks.connect;
|
||||
annotate(
|
||||
`Skipping cluster test, known break since ${publishedLib.version} from #${pr}: ${reason} Impact: ${impact}`
|
||||
);
|
||||
console.log(`PASS: link messages match ${publishedLib.version} apart from known breaks`);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -5,17 +5,17 @@ import {
|
||||
} from "../scripts/version-compat-breaks.js";
|
||||
|
||||
|
||||
describe("version compatibility known breaks", function() {
|
||||
describe("scripts/version-compat-breaks", function() {
|
||||
const breaks = {
|
||||
"2.0.3": {
|
||||
messages: {
|
||||
"request ListedRequest": { pr: 1, reason: "Listed." },
|
||||
"event UnchangedEvent": { pr: 2, reason: "Unchanged." },
|
||||
"request ListedRequest": { pr: 1, reason: "Listed.", impact: "None." },
|
||||
"event UnchangedEvent": { pr: 2, reason: "Unchanged.", impact: "None." },
|
||||
},
|
||||
connect: { pr: 3, reason: "Cannot connect." },
|
||||
connect: { pr: 3, reason: "Cannot connect.", impact: "None." },
|
||||
},
|
||||
"2.0.2": {
|
||||
messages: { "request OldRequest": { pr: 4, reason: "Old." } },
|
||||
messages: { "request OldRequest": { pr: 4, reason: "Old.", impact: "None." } },
|
||||
},
|
||||
};
|
||||
|
||||
@@ -27,24 +27,30 @@ describe("version compatibility known breaks", function() {
|
||||
validateBreaks(breaks);
|
||||
validateBreaks({});
|
||||
});
|
||||
it("should require a pr number and reason", function() {
|
||||
it("should require a pr number, reason and impact", function() {
|
||||
assert.throws(
|
||||
() => validateBreaks({ "1.0.0": { messages: { "request A": { reason: "A." } } } }),
|
||||
/must have a pr number/
|
||||
);
|
||||
assert.throws(
|
||||
() => validateBreaks({ "1.0.0": { messages: { "request A": { pr: 1, reason: " " } } } }),
|
||||
/must have a reason/
|
||||
/is missing reason/
|
||||
);
|
||||
assert.throws(
|
||||
() => validateBreaks({ "1.0.0": { messages: { "request A": { pr: 1, reason: "A." } } } }),
|
||||
/is missing impact/
|
||||
);
|
||||
assert.throws(
|
||||
() => validateBreaks({ "1.0.0": { connect: { pr: 1 } } }),
|
||||
/connect must have a reason/
|
||||
/connect is missing reason/
|
||||
);
|
||||
});
|
||||
it("should reject unknown properties and bad message names", function() {
|
||||
assert.throws(() => validateBreaks({ "1.0.0": { connected: false } }), /unknown properties connected/);
|
||||
assert.throws(
|
||||
() => validateBreaks({ "1.0.0": { messages: { "request A": { pr: 1, reason: "A.", x: 1 } } } }),
|
||||
() => validateBreaks({
|
||||
"1.0.0": { messages: { "request A": { pr: 1, reason: "A.", impact: "A.", x: 1 } } },
|
||||
}),
|
||||
/unknown properties x/
|
||||
);
|
||||
assert.throws(
|
||||
@@ -58,7 +64,7 @@ describe("version compatibility known breaks", function() {
|
||||
it("should pick the entry for the published version", function() {
|
||||
const selected = selectBreaks(breaks, "2.0.3");
|
||||
assert.deepEqual([...selected.messages.keys()], ["request ListedRequest", "event UnchangedEvent"]);
|
||||
assert.deepEqual(selected.connect, { pr: 3, reason: "Cannot connect." });
|
||||
assert.deepEqual(selected.connect, { pr: 3, reason: "Cannot connect.", impact: "None." });
|
||||
assert.deepEqual(selected.ignored, ["2.0.2"]);
|
||||
});
|
||||
it("should ignore every entry when the version is not listed", function() {
|
||||
|
||||
Reference in new issue
Block a user