docs: add GOVERNANCE.md with Discussion-first feature process#2
Conversation
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #2 (community)
Summary
GOVERNANCE.md (new file, 136 lines) introduces a Discussion-first feature process for the Octo project, modeled on Rust RFC / Vue RFC / Kubernetes KEP. The document is well-structured and the prose is clear. All external references (org repos octo-server / octo-web, the org-level Projects URL, Mininglamp-OSS/.github/CODE_OF_CONDUCT.md, the Ideas and Announcements discussion categories) resolve correctly.
The PR ships real value as-is. The comments below are governance-completeness suggestions, not blockers.
Verdict
COMMENTED — non-blocking suggestions. Recommend a follow-up PR (or amendments to this one) to close the gaps in §2.
1. Findings
1.1 Single-maintainer reality vs. plural-maintainer language (P1)
GOVERNANCE.md:9-19
The Maintainers table lists exactly one person (@lml2468, Project Lead). The rest of the document uses plural "Maintainers" throughout ("Maintainers are responsible for…", "maintainers act autonomously", "Maintainers make the final call when the community cannot converge"). With N=1, several clauses are effectively no-ops:
GOVERNANCE.md:118— "maintainers act autonomously" reads as collective decision-making but is currently sole-decision-maker.GOVERNANCE.md:107— "Maintainers make the final call when the community cannot converge on its own" — there is no tie to break.
This is not wrong, but it leaves readers (and future contributors) unclear whether Octo intends a maintainer collective or a Project Lead / BDFL model. Recommend either:
- Be explicit: a one-line note such as "Octo currently operates with a single Project Lead; additional maintainers will be added as the project grows" — or —
- Add a tie-breaking rule that activates once N>1 (e.g., "If maintainers cannot reach consensus, the Project Lead has final authority" — or majority vote, or lazy consensus, etc.).
1.2 Missing: process for adding/removing maintainers (P1)
GOVERNANCE.md defines who maintainers are and what they do, but not how someone becomes one or how someone steps down / is removed. This is one of the most common questions in early-stage OSS governance and one of the most common sources of conflict later. Recommend a short "Becoming a Maintainer" subsection covering, at minimum:
- Eligibility signal (e.g., sustained contribution, demonstrated judgment in reviews / Discussions)
- Nomination mechanism (self-nominate vs. existing-maintainer nominate)
- Approval rule (current Project Lead decides; or unanimous existing maintainers)
- Off-boarding (voluntary step-down, inactivity threshold, removal procedure)
Models worth borrowing:
- Kubernetes: documented contributor ladder
- Rust: team-based with explicit promotion path
- Vue: team membership criteria in their CONTRIBUTING
1.3 Ambiguous semantics of "Maintainer closes the Discussion" (P2)
GOVERNANCE.md:38-40, GOVERNANCE.md:99-104
The flow diagram uses "Maintainer closes Discussion" to mean "consensus reached → convert to Issue". On GitHub, closing a Discussion is also the action used to reject an Idea or mark it as out-of-scope. A reader looking at a closed Discussion has no way to tell which happened.
Two cheap fixes:
- Recommend that the closing comment must link to the tracking Issue (or explicitly state "won't fix" / "deferred"). Suggest standardizing close reasons via the GitHub Discussion close-with-reason feature if available in this category.
- Or use a label/tag convention (e.g.,
accepted,declined,deferred) before/after closing.
Either is worth one sentence in §"Reaching Consensus".
1.4 No path for breaking deadlock when there is no maintainer convergence (P2)
GOVERNANCE.md:107 says "Maintainers make the final call when the community cannot converge on its own." This handles community ↔ maintainer disagreement, but not maintainer ↔ maintainer disagreement (relevant once §1.2 is fixed). Tie this back to whichever model §1.1 lands on.
1.5 Discussion template structure is a placeholder code block (P2)
GOVERNANCE.md:81-103
The template lives inside a fenced code block. Contributors who copy it into a new Discussion will paste a code block, which renders the headings as plain text (no h2 styling, no anchor links). Two options:
- Move the template out of the code fence so the four headings render as actual
##. Code-fenced examples are useful in tutorials but counter-productive in templates meant to be copy-pasted as content. - Or — better — add a Discussion category template (
.github/DISCUSSION_TEMPLATE/feature.ymlor.md) so this structure is auto-applied when contributors open a new Discussion inIdeas. ThenGOVERNANCE.mdcan simply reference the auto-template. This is the canonical mechanism on GitHub and removes the copy-paste step entirely.
1.6 "More than a few days of engineering effort" criterion is unfalsifiable (P3)
GOVERNANCE.md:73 — As written, contributors can't self-classify. Either drop this bullet (the other four criteria already capture significance), or replace with something measurable, e.g., "touches more than one repo" or "would alter a public API". Not a blocker; the closing line ("If you are unsure, open a Discussion — maintainers will guide you") is a reasonable safety valve.
1.7 Amendments section is light on process (P3)
GOVERNANCE.md:131-134 — "This document may be updated by maintainers. Significant changes will be announced…" does not specify what counts as significant, who proposes amendments, or whether non-maintainer contributors can propose them via PR. Other governance docs typically require: PR + announcement period + maintainer approval. With N=1 this is moot, but worth firming up before adding a second maintainer.
1.8 Minor: cross-reference to CONTRIBUTING.md (P3)
The PR description says CONTRIBUTING.md is a companion update in a separate PR. Once that PR lands, GOVERNANCE.md should link to it (e.g., from the "Minor improvements, bug fixes…" sentence at GOVERNANCE.md:75) so contributors who land here know where to find the day-to-day flow. Not a blocker for this PR.
1.9 Nit: ASCII flow diagram doesn't render in some markdown viewers (P3)
GOVERNANCE.md:24-58 — The ASCII art renders fine on GitHub web but may be misaligned in some terminals / mobile viewers / non-monospace contexts. Optional: replace with a Mermaid flowchart block; GitHub renders Mermaid natively and it stays accessible to screen readers. Pure polish — not required.
2. Recommendations (prioritized)
- (P1) Either declare single-maintainer reality explicitly, or land §1.2 (becoming-a-maintainer) before this doc gets quoted in real disputes. Pick one of the two clarifications in §1.1.
- (P1) Add a "Becoming a Maintainer" subsection (§1.2). Three to five sentences is sufficient at this stage.
- (P2) Clarify what "close the Discussion" means and how readers distinguish accepted / declined / deferred outcomes (§1.3).
- (P2) Add a Discussion category template under
.github/DISCUSSION_TEMPLATE/to eliminate the copy-paste step in §"How to Write a Feature Discussion" (§1.5). - (P3) Tighten or remove the "few days of engineering effort" criterion (§1.6).
- (P3) Cross-link to CONTRIBUTING.md once the companion PR lands (§1.8).
None of the above blocks merge. Items (1) and (2) are the highest-leverage; everything else can be follow-up issues.
3. Additional Observations
- The references to Rust RFC / Vue RFC / Kubernetes KEP are well-chosen but aspirational — those projects also have heavy procedural overhead (assigned reviewers, time-bound comment periods, FCP windows). Octo's lighter "Discussion → consensus → Issue" flow is appropriate for current scale; flagging only so the team is aware of the gap if/when scale grows.
- No conflict-of-interest / recusal policy. Standard in larger projects (e.g., a maintainer should not solo-merge their own PR for a significant feature). Worth adding when N>1 maintainers exist.
- No mention of private security disclosure.
Mininglamp-OSS/.github/SECURITY.mdexists at the org level and covers this, so it's not strictly missing — but a one-line pointer inGOVERNANCE.mdwould help readers who land here first.
4. Verification Checklist
- ✅ All external links resolve (
octo-server,octo-web,Mininglamp-OSS/.github/CODE_OF_CONDUCT.md, Octo Board projects URL). - ✅ Referenced discussion categories (
Ideas,Announcements) exist onMininglamp-OSS/community. - ✅ Discussion #1 exists and matches the "governance gap" reference in the PR description.
- ✅ Markdown renders cleanly on GitHub (tables, fenced blocks, headings, blockquote).
- ✅ No typos or broken anchors detected.
⚠️ CompanionCONTRIBUTING.mdPR not yet present in this repo at review time — no cross-link possible until it lands.
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #2 (community)
Summary
GOVERNANCE.md (new file, 136 lines) introduces a Discussion-first feature process for the Octo project, modeled on Rust RFC / Vue RFC / Kubernetes KEP. The document is well-structured and the prose is clear. All external references (org repos octo-server / octo-web, the org-level Projects URL, Mininglamp-OSS/.github/CODE_OF_CONDUCT.md, the Ideas and Announcements discussion categories) resolve correctly.
The PR ships real value as-is. The comments below are governance-completeness suggestions, not blockers.
Verdict
COMMENTED — non-blocking suggestions. Recommend a follow-up PR (or amendments to this one) to close the gaps in §2.
1. Findings
1.1 Single-maintainer reality vs. plural-maintainer language (P1)
GOVERNANCE.md:9-19
The Maintainers table lists exactly one person (@lml2468, Project Lead). The rest of the document uses plural "Maintainers" throughout ("Maintainers are responsible for…", "maintainers act autonomously", "Maintainers make the final call when the community cannot converge"). With N=1, several clauses are effectively no-ops:
GOVERNANCE.md:118— "maintainers act autonomously" reads as collective decision-making but is currently sole-decision-maker.GOVERNANCE.md:107— "Maintainers make the final call when the community cannot converge on its own" — there is no tie to break.
This is not wrong, but it leaves readers (and future contributors) unclear whether Octo intends a maintainer collective or a Project Lead / BDFL model. Recommend either:
- Be explicit: a one-line note such as "Octo currently operates with a single Project Lead; additional maintainers will be added as the project grows" — or —
- Add a tie-breaking rule that activates once N>1 (e.g., "If maintainers cannot reach consensus, the Project Lead has final authority" — or majority vote, or lazy consensus, etc.).
1.2 Missing: process for adding/removing maintainers (P1)
GOVERNANCE.md defines who maintainers are and what they do, but not how someone becomes one or how someone steps down / is removed. This is one of the most common questions in early-stage OSS governance and one of the most common sources of conflict later. Recommend a short "Becoming a Maintainer" subsection covering, at minimum:
- Eligibility signal (e.g., sustained contribution, demonstrated judgment in reviews / Discussions)
- Nomination mechanism (self-nominate vs. existing-maintainer nominate)
- Approval rule (current Project Lead decides; or unanimous existing maintainers)
- Off-boarding (voluntary step-down, inactivity threshold, removal procedure)
Models worth borrowing:
- Kubernetes: documented contributor ladder
- Rust: team-based with explicit promotion path
- Vue: team membership criteria in their CONTRIBUTING
1.3 Ambiguous semantics of "Maintainer closes the Discussion" (P2)
GOVERNANCE.md:38-40, GOVERNANCE.md:99-104
The flow diagram uses "Maintainer closes Discussion" to mean "consensus reached → convert to Issue". On GitHub, closing a Discussion is also the action used to reject an Idea or mark it as out-of-scope. A reader looking at a closed Discussion has no way to tell which happened.
Two cheap fixes:
- Recommend that the closing comment must link to the tracking Issue (or explicitly state "won't fix" / "deferred"). Suggest standardizing close reasons via the GitHub Discussion close-with-reason feature if available in this category.
- Or use a label/tag convention (e.g.,
accepted,declined,deferred) before/after closing.
Either is worth one sentence in §"Reaching Consensus".
1.4 No path for breaking deadlock when there is no maintainer convergence (P2)
GOVERNANCE.md:107 says "Maintainers make the final call when the community cannot converge on its own." This handles community ↔ maintainer disagreement, but not maintainer ↔ maintainer disagreement (relevant once §1.2 is fixed). Tie this back to whichever model §1.1 lands on.
1.5 Discussion template structure is a placeholder code block (P2)
GOVERNANCE.md:81-103
The template lives inside a fenced code block. Contributors who copy it into a new Discussion will paste a code block, which renders the headings as plain text (no h2 styling, no anchor links). Two options:
- Move the template out of the code fence so the four headings render as actual
##. Code-fenced examples are useful in tutorials but counter-productive in templates meant to be copy-pasted as content. - Or — better — add a Discussion category template (
.github/DISCUSSION_TEMPLATE/feature.ymlor.md) so this structure is auto-applied when contributors open a new Discussion inIdeas. ThenGOVERNANCE.mdcan simply reference the auto-template. This is the canonical mechanism on GitHub and removes the copy-paste step entirely.
1.6 "More than a few days of engineering effort" criterion is unfalsifiable (P3)
GOVERNANCE.md:73 — As written, contributors can't self-classify. Either drop this bullet (the other four criteria already capture significance), or replace with something measurable, e.g., "touches more than one repo" or "would alter a public API". Not a blocker; the closing line ("If you are unsure, open a Discussion — maintainers will guide you") is a reasonable safety valve.
1.7 Amendments section is light on process (P3)
GOVERNANCE.md:131-134 — "This document may be updated by maintainers. Significant changes will be announced…" does not specify what counts as significant, who proposes amendments, or whether non-maintainer contributors can propose them via PR. Other governance docs typically require: PR + announcement period + maintainer approval. With N=1 this is moot, but worth firming up before adding a second maintainer.
1.8 Minor: cross-reference to CONTRIBUTING.md (P3)
The PR description says CONTRIBUTING.md is a companion update in a separate PR. Once that PR lands, GOVERNANCE.md should link to it (e.g., from the "Minor improvements, bug fixes…" sentence at GOVERNANCE.md:75) so contributors who land here know where to find the day-to-day flow. Not a blocker for this PR.
1.9 Nit: ASCII flow diagram doesn't render in some markdown viewers (P3)
GOVERNANCE.md:24-58 — The ASCII art renders fine on GitHub web but may be misaligned in some terminals / mobile viewers / non-monospace contexts. Optional: replace with a Mermaid flowchart block; GitHub renders Mermaid natively and it stays accessible to screen readers. Pure polish — not required.
2. Recommendations (prioritized)
- (P1) Either declare single-maintainer reality explicitly, or land §1.2 (becoming-a-maintainer) before this doc gets quoted in real disputes. Pick one of the two clarifications in §1.1.
- (P1) Add a "Becoming a Maintainer" subsection (§1.2). Three to five sentences is sufficient at this stage.
- (P2) Clarify what "close the Discussion" means and how readers distinguish accepted / declined / deferred outcomes (§1.3).
- (P2) Add a Discussion category template under
.github/DISCUSSION_TEMPLATE/to eliminate the copy-paste step in §"How to Write a Feature Discussion" (§1.5). - (P3) Tighten or remove the "few days of engineering effort" criterion (§1.6).
- (P3) Cross-link to CONTRIBUTING.md once the companion PR lands (§1.8).
None of the above blocks merge. Items (1) and (2) are the highest-leverage; everything else can be follow-up issues.
3. Additional Observations
- The references to Rust RFC / Vue RFC / Kubernetes KEP are well-chosen but aspirational — those projects also have heavy procedural overhead (assigned reviewers, time-bound comment periods, FCP windows). Octo's lighter "Discussion → consensus → Issue" flow is appropriate for current scale; flagging only so the team is aware of the gap if/when scale grows.
- No conflict-of-interest / recusal policy. Standard in larger projects (e.g., a maintainer should not solo-merge their own PR for a significant feature). Worth adding when N>1 maintainers exist.
- No mention of private security disclosure.
Mininglamp-OSS/.github/SECURITY.mdexists at the org level and covers this, so it's not strictly missing — but a one-line pointer inGOVERNANCE.mdwould help readers who land here first.
4. Verification Checklist
- ✅ All external links resolve (
octo-server,octo-web,Mininglamp-OSS/.github/CODE_OF_CONDUCT.md, Octo Board projects URL). - ✅ Referenced discussion categories (
Ideas,Announcements) exist onMininglamp-OSS/community. - ✅ Discussion #1 exists and matches the "governance gap" reference in the PR description.
- ✅ Markdown renders cleanly on GitHub (tables, fenced blocks, headings, blockquote).
- ✅ No typos or broken anchors detected.
⚠️ CompanionCONTRIBUTING.mdPR not yet present in this repo at review time — no cross-link possible until it lands.
d138755 to
e649151
Compare
|
Thanks @yujiawei for the thorough review! All P1 and P2 items have been addressed in the amended commit: P1 — Single-maintainer reality (§1.1) P1 — Becoming a Maintainer (§1.2) P2 — Ambiguous close semantics (§1.3) P2 — Discussion template as code block (§1.5) P3 — Unfalsifiable effort criterion (§1.6) P3 — Cross-link to CONTRIBUTING.md (§1.8) Security pointer (Additional Observations) P3 — Mermaid diagram (§1.9) P3 items §1.7 (Amendments process detail) has also been expanded — the section now specifies who proposes, what counts as significant, and that non-maintainer contributors may propose via PR. |
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #2 (community)
Thanks for putting this together — overall the document is well-structured, references mature precedents (Rust RFC, Vue RFC, Kubernetes KEP), and lays out a reasonable Discussion-first flow for a single-maintainer project. Linked external files (CONTRIBUTING.md, SECURITY.md, CODE_OF_CONDUCT.md in Mininglamp-OSS/.github) and the Octo Board URL all resolve.
I am leaving this as a comment review rather than blocking, but there is one functional issue (P1) that should be fixed before merge, plus a few smaller items.
Findings
P1 — Discussion template will not auto-apply as claimed
Files: .github/DISCUSSION_TEMPLATE/feature_request.md (new), GOVERNANCE.md lines 79–80
GOVERNANCE.md tells contributors:
"Open a new Discussion in the Ideas category. A structured template is pre-filled automatically when you start a new Discussion there."
This will not happen with the file as committed. Per the GitHub docs on discussion category forms:
- The file must be YAML (
.yml), not Markdown (.md). The current file is.md. - The filename must match the category slug. The category in question is
ideas, so the file would need to be.github/DISCUSSION_TEMPLATE/ideas.yml.feature_request.mddoes not match any of the repo's category slugs (announcements,general,ideas,polls,q-a,show-and-tell). - The schema is GitHub's form schema (
name,description,body:as a list oftype: textareablocks withid/attributes), not free-form Markdown withtitle/labels/bodyfrontmatter.
As written, the file is a Markdown document that GitHub will simply ignore for auto-population. Contributors who follow the GOVERNANCE.md instructions will land on a blank Discussion form.
Suggested fix: convert to a YAML category form named ideas.yml, e.g.:
title: "[Feature] "
labels: []
body:
- type: markdown
attributes:
value: |
Thanks for proposing a feature for Octo. Please fill in the sections below.
- type: textarea
id: problem
attributes:
label: Problem
description: What problem are you solving? Who is affected, and how often?
validations:
required: true
- type: textarea
id: proposed-solution
attributes:
label: Proposed Solution
validations:
required: true
- type: textarea
id: alternatives
attributes:
label: Alternatives Considered
- type: textarea
id: additional-context
attributes:
label: Additional ContextVerify after merge by clicking "New discussion → Ideas" and confirming the form pre-fills.
If the intent is genuinely to keep a free-form Markdown copy of the template (e.g., for documentation), that is fine — but in that case GOVERNANCE.md should not promise auto-population, and the file name should reflect that it is a reference document, not a category form.
P2 — "Emeritus status" is referenced but not defined
File: GOVERNANCE.md, lines 33–34
"A maintainer inactive for 6+ months may be moved to Emeritus status after a notice period."
Emeritus is introduced here for the first and only time. The doc doesn't say what an Emeritus maintainer is allowed to do (merge rights revoked? listed in a separate table? still credited?). Either define the term in a short bullet under Maintainers, or drop the proper-noun framing and just say "removed from the active maintainer list."
P2 — "Closes the governance gap identified in discussion #1" is a tenuous link
File: PR body
discussion#1 is a concrete feature request (channel public visibility), not a meta-discussion about governance. It currently has zero comments, so a reader following the link will not find any record of the governance gap being identified there. This is a PR-description nit (does not affect the merged content), but if the intent is to credit the moment the gap surfaced, consider linking to a governance-specific Discussion or simply rewording to "Establishes the governance process previously absent in this organization."
P3 — Mermaid line breaks
File: GOVERNANCE.md, lines 47–55
The Mermaid flowchart uses \n inside node labels (e.g. B[Open a Discussion\nin community · Ideas]). GitHub's Mermaid renderer (v10+) does support \n, so this should work today, but <br/> is more portable across Mermaid versions and tooling. Optional change.
P3 — Amendments process is lighter than the feature process it governs
File: GOVERNANCE.md, lines 121–127
The Amendments section says material governance changes only need to be announced in the Announcements category. Significant features, by contrast, require a Discussion in Ideas with consensus before becoming an Issue. It is internally inconsistent for governance changes (which alter the rules for everyone) to have a lower bar than a single feature. Consider requiring material amendments to themselves go through a Discussion in Ideas (or a dedicated governance category), with the announcement happening after merge.
This is a judgment call and reasonable people will disagree — flagging for the author's consideration, not as a blocker.
P3 — English-only document, one Chinese name
File: GOVERNANCE.md, line 11
The maintainer table lists 李梦林 while the rest of the document is in English. Not wrong, but if Octo intends an international contributor base, consider listing the romanized form alongside (e.g., Menglin Li (李梦林)) for searchability and pronounceability. Author preference.
Approvals / Things that look good
- ✅ Discussion-first model is clearly explained and inspired by sound precedents.
- ✅ The "What requires a Discussion?" criteria are concrete and testable (new user-facing concept, breaking change, multi-repo coordination, etc.) — no fuzzy "ask the maintainer" loophole.
- ✅ "Consensus does not mean unanimous agreement" is explicitly defined, which avoids a common pitfall.
- ✅ Single-Project-Lead reality is acknowledged honestly rather than papered over with a fictional committee.
- ✅ Off-boarding and self-step-down procedures included — frequently omitted in early-stage governance docs.
- ✅ Security exception correctly redirects to private disclosure.
- ✅ All cross-repo links (
Mininglamp-OSS/.githubforCONTRIBUTING.md,SECURITY.md,CODE_OF_CONDUCT.md; org Projects URL) resolve.
Recommendation
Address the P1 (template format/filename) before merge so the GOVERNANCE.md promise of an auto-filled template is actually true. The P2/P3 items are non-blocking polish.
Summary
Adds
GOVERNANCE.mdto document Octo's governance model and feature development process.What's included
Related
.github/CONTRIBUTING.md(separate PR)