mirror of
https://github.com/grumpycoders/pcsx-redux.git
synced 2026-10-12 02:15:13 +00:00
RFC.md and the rfc-moratorium check
A pull request labeled rfc cannot merge for seven days after the label was last applied. Same workflow as pcsx-redux/nugget.
This commit is contained in:
@@ -0,0 +1,8 @@
|
||||
# Who is mentioned when the rfc label goes on a pull request, by the paths it
|
||||
# touches. One pattern per line, then the GitHub handles to mention.
|
||||
# * every change
|
||||
# dir/ anything under dir
|
||||
# path that exact file
|
||||
# The pull request's author is never mentioned.
|
||||
|
||||
* @nicolasnoble
|
||||
@@ -0,0 +1,121 @@
|
||||
'use strict';
|
||||
|
||||
// Sets the rfc-moratorium commit status on open pull requests and rewrites
|
||||
// the pinned "Open RFCs" issue. Run by .github/workflows/rfc.yml; see RFC.md.
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const LABEL = 'rfc';
|
||||
const CONTEXT = 'rfc-moratorium';
|
||||
const WINDOW_MS = 7 * 24 * 60 * 60 * 1000;
|
||||
const INDEX_TITLE = 'Open RFCs';
|
||||
|
||||
function stamp(ms) {
|
||||
return new Date(ms).toISOString().slice(0, 16).replace('T', ' ') + ' UTC';
|
||||
}
|
||||
|
||||
// When the rfc label was last applied, from the issue events, or null.
|
||||
function labeledAt(events) {
|
||||
let at = null;
|
||||
for (const e of events) {
|
||||
if (e.event !== 'labeled' || !e.label || e.label.name !== LABEL) continue;
|
||||
const t = Date.parse(e.created_at);
|
||||
if (at === null || t > at) at = t;
|
||||
}
|
||||
return at;
|
||||
}
|
||||
|
||||
function verdict(labels, events, now) {
|
||||
if (!labels.includes(LABEL)) return { state: 'success', description: 'Not an RFC' };
|
||||
const at = labeledAt(events);
|
||||
if (at === null) return { state: 'failure', description: 'Labeled rfc, but no labeled event found' };
|
||||
const notBefore = at + WINDOW_MS;
|
||||
if (now < notBefore) {
|
||||
return { state: 'failure', description: `RFC window open, merge not before ${stamp(notBefore)}`, notBefore };
|
||||
}
|
||||
return { state: 'success', description: `RFC window closed ${stamp(notBefore)}`, notBefore };
|
||||
}
|
||||
|
||||
function parseStakeholders(text) {
|
||||
const rules = [];
|
||||
for (const raw of text.split('\n')) {
|
||||
const line = raw.replace(/#.*/, '').trim();
|
||||
if (!line) continue;
|
||||
const [pattern, ...handles] = line.split(/\s+/);
|
||||
rules.push({ pattern, handles: handles.map((h) => h.replace(/^@/, '')) });
|
||||
}
|
||||
return rules;
|
||||
}
|
||||
|
||||
function stakeholders(rules, files, author) {
|
||||
const out = new Set();
|
||||
for (const { pattern, handles } of rules) {
|
||||
const hit = files.some((f) =>
|
||||
pattern === '*' || (pattern.endsWith('/') ? f.startsWith(pattern) : f === pattern));
|
||||
if (hit) handles.forEach((h) => out.add(h));
|
||||
}
|
||||
out.delete(author);
|
||||
return [...out];
|
||||
}
|
||||
|
||||
function indexBody(owner, repo, rows) {
|
||||
const head = 'Pull requests carrying the `rfc` label and the earliest time each can merge. ' +
|
||||
`The RFC workflow rewrites this issue, see [RFC.md](https://github.com/${owner}/${repo}/blob/main/RFC.md). ` +
|
||||
'Comment on the pull request itself.\n\n';
|
||||
if (rows.length === 0) return head + 'No open RFCs.\n';
|
||||
rows.sort((a, b) => a.notBefore - b.notBefore);
|
||||
return head + '| Pull request | Merge not before |\n|---|---|\n' +
|
||||
rows.map((r) => `| #${r.number} ${r.title.replace(/\|/g, '\\|')} | ${r.notBefore ? stamp(r.notBefore) : 'unknown'} |`).join('\n') + '\n';
|
||||
}
|
||||
|
||||
async function run({ github, context, core, now = Date.now() }) {
|
||||
const { owner, repo } = context.repo;
|
||||
const target = `https://github.com/${owner}/${repo}/blob/main/RFC.md`;
|
||||
const prs = await github.paginate(github.rest.pulls.list, { owner, repo, state: 'open', per_page: 100 });
|
||||
const rows = [];
|
||||
|
||||
for (const pr of prs) {
|
||||
const labels = pr.labels.map((l) => l.name);
|
||||
const events = labels.includes(LABEL)
|
||||
? await github.paginate(github.rest.issues.listEvents, { owner, repo, issue_number: pr.number, per_page: 100 })
|
||||
: [];
|
||||
const v = verdict(labels, events, now);
|
||||
if (labels.includes(LABEL)) rows.push({ number: pr.number, title: pr.title, notBefore: v.notBefore });
|
||||
|
||||
const { data: current } = await github.rest.repos.listCommitStatusesForRef({
|
||||
owner, repo, ref: pr.head.sha, per_page: 100,
|
||||
});
|
||||
const last = current.find((s) => s.context === CONTEXT);
|
||||
if (last && last.state === v.state && last.description === v.description) continue;
|
||||
await github.rest.repos.createCommitStatus({
|
||||
owner, repo, sha: pr.head.sha, state: v.state, context: CONTEXT, description: v.description, target_url: target,
|
||||
});
|
||||
core.info(`#${pr.number}: ${v.state}, ${v.description}`);
|
||||
}
|
||||
|
||||
const payload = context.payload;
|
||||
if (context.eventName === 'pull_request_target' && payload.action === 'labeled' && payload.label.name === LABEL) {
|
||||
const pr = payload.pull_request;
|
||||
const files = (await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr.number, per_page: 100 }))
|
||||
.map((f) => f.filename);
|
||||
const text = fs.readFileSync(path.join(__dirname, '..', 'rfc-stakeholders'), 'utf8');
|
||||
const who = stakeholders(parseStakeholders(text), files, pr.user.login);
|
||||
const row = rows.find((r) => r.number === pr.number);
|
||||
const body = `This is now an RFC: it cannot merge before ${row && row.notBefore ? stamp(row.notBefore) : 'the window closes'}, ` +
|
||||
'so anyone who depends on what it changes has a week to comment. See [RFC.md](' + target + ').' +
|
||||
(who.length ? '\n\n' + who.map((h) => '@' + h).join(' ') + ', this touches code you depend on.' : '');
|
||||
await github.rest.issues.createComment({ owner, repo, issue_number: pr.number, body });
|
||||
}
|
||||
|
||||
const issues = await github.paginate(github.rest.issues.listForRepo, { owner, repo, state: 'open', per_page: 100 });
|
||||
const index = issues.find((i) => !i.pull_request && i.title === INDEX_TITLE);
|
||||
const body = indexBody(owner, repo, rows);
|
||||
if (!index) {
|
||||
core.warning(`No open issue titled "${INDEX_TITLE}", so no index to update.`);
|
||||
} else if (index.body !== body) {
|
||||
await github.rest.issues.update({ owner, repo, issue_number: index.number, body });
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = { run, verdict, labeledAt, parseStakeholders, stakeholders, indexBody, WINDOW_MS };
|
||||
@@ -0,0 +1,103 @@
|
||||
'use strict';
|
||||
|
||||
// node --test .github/scripts/rfc.test.js ; run by .github/workflows/rfc-script.yml.
|
||||
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert');
|
||||
const rfc = require('./rfc.js');
|
||||
|
||||
const DAY = 24 * 60 * 60 * 1000;
|
||||
const t0 = Date.parse('2026-09-01T12:00:00Z');
|
||||
const on = (at) => ({ event: 'labeled', label: { name: 'rfc' }, created_at: new Date(at).toISOString() });
|
||||
const off = (at) => ({ event: 'unlabeled', label: { name: 'rfc' }, created_at: new Date(at).toISOString() });
|
||||
|
||||
test('no label passes', () => {
|
||||
assert.strictEqual(rfc.verdict(['bug'], [on(t0)], t0).state, 'success');
|
||||
});
|
||||
|
||||
test('fails inside the window, passes after it', () => {
|
||||
assert.strictEqual(rfc.verdict(['rfc'], [on(t0)], t0 + 7 * DAY - 1).state, 'failure');
|
||||
assert.strictEqual(rfc.verdict(['rfc'], [on(t0)], t0 + 7 * DAY).state, 'success');
|
||||
});
|
||||
|
||||
test('re-applying the label restarts the clock', () => {
|
||||
const events = [on(t0), off(t0 + 6 * DAY), on(t0 + 6 * DAY + 1)];
|
||||
const v = rfc.verdict(['rfc'], events, t0 + 8 * DAY);
|
||||
assert.strictEqual(v.state, 'failure');
|
||||
assert.strictEqual(v.notBefore, t0 + 13 * DAY + 1);
|
||||
});
|
||||
|
||||
test('other labels do not start the clock', () => {
|
||||
const other = { event: 'labeled', label: { name: 'bug' }, created_at: new Date(t0 + 5 * DAY).toISOString() };
|
||||
assert.strictEqual(rfc.verdict(['rfc'], [on(t0), other], t0 + 7 * DAY).state, 'success');
|
||||
});
|
||||
|
||||
test('label with no event fails', () => {
|
||||
assert.strictEqual(rfc.verdict(['rfc'], [], t0).state, 'failure');
|
||||
});
|
||||
|
||||
test('description fits a commit status', () => {
|
||||
assert.ok(rfc.verdict(['rfc'], [on(t0)], t0).description.length <= 140);
|
||||
});
|
||||
|
||||
test('stakeholders by path, author left out', () => {
|
||||
const rules = rfc.parseStakeholders('# c\n* @a\nmonitor/ @b @a\nopenbios/x.s @c\n');
|
||||
assert.deepStrictEqual(rfc.stakeholders(rules, ['psyqo/x.h'], 'z'), ['a']);
|
||||
assert.deepStrictEqual(rfc.stakeholders(rules, ['monitor/monitor.c'], 'a'), ['b']);
|
||||
assert.deepStrictEqual(rfc.stakeholders(rules, ['openbios/x.s'], 'z').sort(), ['a', 'c']);
|
||||
assert.deepStrictEqual(rfc.stakeholders(rules, ['openbios/x.s.bak'], 'z'), ['a']);
|
||||
});
|
||||
|
||||
test('the real stakeholders file parses', () => {
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const rules = rfc.parseStakeholders(fs.readFileSync(path.join(__dirname, '..', 'rfc-stakeholders'), 'utf8'));
|
||||
assert.deepStrictEqual(rfc.stakeholders(rules, ['src/core/sstate.cc'], 'x'), ['nicolasnoble']);
|
||||
});
|
||||
|
||||
test('run sets statuses, comments on labeling, rewrites the index', async () => {
|
||||
const calls = [];
|
||||
const prs = [
|
||||
{ number: 37, title: 'monitor: protocol v3', labels: [{ name: 'rfc' }], head: { sha: 'aaa' }, user: { login: 'rixnobis' } },
|
||||
{ number: 38, title: 'fix', labels: [], head: { sha: 'bbb' }, user: { login: 'x' } },
|
||||
];
|
||||
const lists = {
|
||||
pulls: prs,
|
||||
events: [on(t0)],
|
||||
files: [{ filename: 'monitor/monitor.c' }],
|
||||
issues: [{ number: 47, title: 'Open RFCs', body: 'old' }],
|
||||
};
|
||||
const github = {
|
||||
paginate: async (fn, args) => fn(args),
|
||||
rest: {
|
||||
pulls: { list: () => lists.pulls, listFiles: () => lists.files },
|
||||
issues: {
|
||||
listEvents: () => lists.events,
|
||||
listForRepo: () => lists.issues,
|
||||
createComment: async (a) => calls.push(['comment', a]),
|
||||
update: async (a) => calls.push(['update', a]),
|
||||
create: async (a) => calls.push(['create', a]),
|
||||
},
|
||||
repos: {
|
||||
listCommitStatusesForRef: async () => ({ data: [] }),
|
||||
createCommitStatus: async (a) => calls.push(['status', a]),
|
||||
},
|
||||
},
|
||||
};
|
||||
const context = {
|
||||
repo: { owner: 'o', repo: 'r' },
|
||||
eventName: 'pull_request_target',
|
||||
payload: { action: 'labeled', label: { name: 'rfc' }, pull_request: prs[0] },
|
||||
};
|
||||
const core = { info() {}, warning() {} };
|
||||
await rfc.run({ github, context, core, now: t0 + DAY });
|
||||
|
||||
const statuses = calls.filter((c) => c[0] === 'status').map((c) => [c[1].sha, c[1].state]);
|
||||
assert.deepStrictEqual(statuses, [['aaa', 'failure'], ['bbb', 'success']]);
|
||||
const comment = calls.find((c) => c[0] === 'comment')[1];
|
||||
assert.match(comment.body, /2026-09-08 12:00 UTC/);
|
||||
assert.match(comment.body, /@nicolasnoble,/);
|
||||
const update = calls.find((c) => c[0] === 'update')[1];
|
||||
assert.strictEqual(update.issue_number, 47);
|
||||
assert.match(update.body, /\| #37 monitor: protocol v3 \| 2026-09-08 12:00 UTC \|/);
|
||||
});
|
||||
@@ -0,0 +1,14 @@
|
||||
name: RFC script
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
paths:
|
||||
- '.github/scripts/rfc*'
|
||||
- '.github/rfc-stakeholders'
|
||||
|
||||
jobs:
|
||||
test:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- run: node --test .github/scripts/rfc.test.js
|
||||
@@ -0,0 +1,36 @@
|
||||
name: RFC
|
||||
|
||||
# Keeps the rfc-moratorium status and the pinned Open RFCs issue current. See
|
||||
# RFC.md. The hourly run is what turns the status green once a window closes.
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [opened, reopened, synchronize, labeled, unlabeled, closed]
|
||||
schedule:
|
||||
- cron: '17 * * * *'
|
||||
workflow_dispatch:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
issues: write
|
||||
pull-requests: write
|
||||
statuses: write
|
||||
|
||||
concurrency:
|
||||
group: rfc
|
||||
cancel-in-progress: false
|
||||
|
||||
jobs:
|
||||
moratorium:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
# This checks out the base branch. The job holds a write token, so it
|
||||
# must never check out or run anything from the pull request.
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
sparse-checkout: .github
|
||||
|
||||
- uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
await require('./.github/scripts/rfc.js').run({ github, context, core });
|
||||
@@ -0,0 +1,46 @@
|
||||
# RFCs
|
||||
|
||||
Some changes commit PCSX-Redux to something its users build on or rely on, and undoing them
|
||||
later means breaking those users. A pull request making such a change gets the `rfc` label and
|
||||
cannot merge for seven days, so the people who depend on it can comment first.
|
||||
|
||||
## What needs one
|
||||
|
||||
- Save state changes that stop older states from loading. A new protobuf field that older
|
||||
states simply load as its default value is not one of these.
|
||||
- New UX: new windows, menus or workflows, changed defaults, removed options.
|
||||
- The Lua API.
|
||||
- Command-line flags.
|
||||
- The GDB server and web server protocols.
|
||||
- File formats.
|
||||
|
||||
A bugfix that makes the code do what it already promises does not need one, and neither do
|
||||
tests or documentation. Who opens the pull request makes no difference. `src/mips` is the
|
||||
[nugget](https://github.com/pcsx-redux/nugget) submodule, which runs its own RFCs; bumping it
|
||||
here is not one. If you are not sure, ask on the pull request or add the label.
|
||||
|
||||
## The window
|
||||
|
||||
While the label is on, the `rfc-moratorium` check fails until seven days after the label was
|
||||
last applied, and `main` requires that check. The check's description gives the time the
|
||||
window closes. If the proposal changes during the window, remove and re-apply the label and say
|
||||
what changed in a comment; the seven days start over. Changes that leave the proposal alone do
|
||||
not restart it.
|
||||
|
||||
Open RFCs are the
|
||||
[open pull requests with the label](https://github.com/grumpycoders/pcsx-redux/pulls?q=is%3Apr+is%3Aopen+label%3Arfc).
|
||||
When the label goes on, the people listed in [.github/rfc-stakeholders](.github/rfc-stakeholders)
|
||||
are mentioned on the pull request.
|
||||
|
||||
## What an RFC has to say
|
||||
|
||||
Before the window closes, the pull request description states what the change costs and who
|
||||
it affects: for a save state change, which versions stop loading; for a UX change, what a user
|
||||
does differently afterwards, with a screenshot; for anything else, the size and speed it costs,
|
||||
measured on a build. An RFC missing that does not merge when the window closes; it waits until
|
||||
it is there.
|
||||
|
||||
## Commenting
|
||||
|
||||
Comment on the pull request. An objection helps most with a use case attached: what you do
|
||||
today that the change would break, or what it would stop you from doing.
|
||||
Reference in New Issue
Block a user