docs: add CODEOWNERS, ADR template, and RFC process#96
Conversation
Introduces three lightweight governance mechanisms: - .github/CODEOWNERS — auto-requests reviewers and (with branch protection) gates merges on sensitive paths: wire protocol, storage trait, auth, CLI, and process docs. - docs/adr/ — adds 0000-template.md and README.md describing when to write an ADR. Renames the two existing ADRs to 0001/0002 so the numbering convention is consistent. - rfcs/ — adds 0000-template.md and README.md describing when an RFC is required (wire protocol, Storage trait, auth, on-disk format, public CLI) and the Draft → Under Review → FCP → Accepted lifecycle. - CONTRIBUTING.md — adds a "When you need an ADR or an RFC" section. - .github/pull_request_template.md — adds a checklist line linking PRs to an ADR or RFC when they touch protected surfaces.
jcshepherd
left a comment
There was a problem hiding this comment.
Nice! I left a few comments/questions around the RFC process (based on lessons learned/observed in the Cassandra world) but overall this is a nice start.
|
|
||
| - API or wire-format changes, with concrete request/response examples | ||
| - Behavioral changes | ||
| - Migration path for existing users |
There was a problem hiding this comment.
Might also add something like "New operational requirements" or similar. Are users going to need understand anything new to use the proposed thingie?
There was a problem hiding this comment.
Added an explicit "New operational requirements" bullet to the Detailed Design section so authors have to think about what an operator learns, configures, monitors, or runs differently after the change lands (new daemons, ports, config keys, metrics, runbooks, dependencies). e1ce936.
|
|
||
| Open an RFC if your change touches any of: | ||
|
|
||
| - DynamoDB wire-protocol response shapes or new API operations |
There was a problem hiding this comment.
What about significant new features in general: e.g., replication?
There was a problem hiding this comment.
Agreed. The trigger list was surface-defined and would have let big subsystems slip through. Added a catch-all bullet: "A significant new feature or subsystem, even if it does not directly touch the surfaces above (for example, replication, multi-region, change data capture, a new auth provider, a new query interface)." e1ce936.
| ## Process | ||
|
|
||
| 1. **Draft.** Copy `0000-template.md` to `rfcs/0000-my-proposal.md` (keep `0000` | ||
| until accepted). Open a PR. The PR is the discussion thread. |
There was a problem hiding this comment.
I haven't looked at GitHub "Discussions" at all, but would it be worth having RFC submitters start a discussion with a link to the PR with the RFC, and have the discussion there, rather than cluttering the PR with potentially long wandering conversations? (The RFC can also have a link back to the discussion, for posterity.)
There was a problem hiding this comment.
For now we are going with PR-as-thread because (a) it keeps line-level review and high-level debate in one place, and (b) we do not yet have enough RFC volume to know whether thread bloat will actually be the problem. I added a sentence noting that a maintainer can move broad design debate to GitHub Discussions if a PR becomes hard to follow, with the PR reserved for line-level comments. We can codify a stricter split if we hit the pain. e1ce936.
|
|
||
| 1. **Draft.** Copy `0000-template.md` to `rfcs/0000-my-proposal.md` (keep `0000` | ||
| until accepted). Open a PR. The PR is the discussion thread. | ||
| 2. **Under Review.** A maintainer assigns reviewers from |
There was a problem hiding this comment.
Specified: minimum two maintainers, including at least one code owner for each surface the RFC affects. For now we are treating the people listed in .github/CODEOWNERS as the maintainer set, which keeps the rule grounded in something concrete without us needing a separate MAINTAINERS.md yet. e1ce936.
| 2. **Under Review.** A maintainer assigns reviewers from | ||
| [`.github/CODEOWNERS`](../.github/CODEOWNERS). Discussion happens inline on | ||
| the PR. | ||
| 3. **Final Comment Period (FCP).** When reviewers are aligned, a maintainer |
There was a problem hiding this comment.
Would any kind of voting mechanism be useful? E.g., minimum of X maintainers vote to accept, and none vote to reject?
There was a problem hiding this comment.
Pulling out the lazy "no objections surface" wording. It concentrated too much call-it discretion in whichever maintainer announced FCP and did not tell a contributor what success looked like. Replaced with an explicit rule: at end of FCP, the RFC is accepted if (a) at least two maintainers have approved, (b) no maintainer has requested changes, and (c) no new substantive objection was raised during the FCP. A new substantive objection resets the 7-day clock. This gives contributors a deterministic finish line and gives any maintainer a real veto without naming any one person as a blocker. e1ce936.
|
One other suggestion: I'd suggest putting rfcs/ under docs/ instead of in the project root. |
- Add "New operational requirements" bullet to the RFC template's Detailed Design section so authors have to surface ops impact (new daemons, ports, config, metrics, runbooks). - Add a catch-all trigger for significant new features and subsystems (e.g., replication, multi-region, CDC) so things that do not touch the listed surfaces still get an RFC. - Specify reviewer count: minimum two maintainers, including at least one code owner for each surface the RFC affects. - Make FCP acceptance criteria explicit: 2 maintainer approvals, no requested-changes, no new substantive objections during FCP; new substantive objection resets the 7-day clock. - Note that PR-as-thread is the default for now, with an out to move long discussions to GitHub Discussions if the PR becomes unwieldy.
c6e1a96 to
e1ce936
Compare
Adds a "Contributing" section to README.md so contributors find the process docs from the project front door, not just from CONTRIBUTING.md. Points at docs/adr/README.md, rfcs/README.md, and .github/CODEOWNERS.
Per @jcshepherd's suggestion, putting rfcs/ under docs/ for consistency with docs/adr/ and to keep the project root cleaner. Updates all cross-references: CODEOWNERS path, README and CONTRIBUTING links, the ADR README's link to RFCs, and the relative paths inside docs/rfcs/README.md (CODEOWNERS, ADR cross-link) and docs/rfcs/0000-template.md (LICENSE).
|
Done in 407c85c. Moved |
Summary
Introduces three lightweight governance mechanisms so contributions to sensitive surfaces (wire protocol, storage trait, auth, on-disk format, public CLI) get the right review and a written record.
.github/CODEOWNERS— auto-requests reviewers and (with branch protection enabled) gates merges on sensitive paths.docs/adr/— adds0000-template.mdand aREADME.mddescribing when to write an ADR. Renames the two existing ADRs to0001-documentation-format.mdand0002-sql-injection-defense.mdso the numbering convention is consistent going forward.docs/rfcs/— adds0000-template.mdand aREADME.mddescribing when an RFC is required and the Draft → Under Review → FCP → Accepted lifecycle.CONTRIBUTING.md— adds a "When you need an ADR or an RFC" section explaining the difference (ADR records a decision, RFC solicits input on a proposal).README.md— adds a "Contributing" section linking to ADRs, RFCs, and CODEOWNERS so contributors find the process from the project front door..github/pull_request_template.md— adds one checklist line so PRs touching protected surfaces link to an ADR or RFC.ADR vs. RFC, in one line each
Why this shape
ExtendDB already had a
docs/adr/folder and two ADRs, but no template, no README, no numbering, and no documented trigger for when one is required. This PR fills those gaps with the lightest mechanism that still creates the discipline.CODEOWNERS is intentionally uniform across paths today (same four owners on every line). The per-path lines are kept so future specialization (e.g., a dedicated storage maintainer) is one edit away.
Activation note (for maintainers)
CODEOWNERS does not gate merges by itself. To enforce, go to Settings → Branches → branch protection rule for
mainand enable "Require review from Code Owners".Follow-ups
.github/CODEOWNERSas the maintainer list. Issue Formalize maintainer set and governance model #117 tracks the eventualMAINTAINERS.md/GOVERNANCE.mdwork that would replace this stopgap.Test plan
docs/adr/README.mdindex links resolvedocs/rfcs/README.mdlinks to.github/CODEOWNERSresolve