doc/src/review_policy: init

[trueNAHO]

Add a generic rejection policy inspired by #2434.

---

• I certify that I have the right to submit this contribution under the MIT license
• Commit messages adhere to Stylix commit conventions
• Theming changes adhere to the Stylix style guide
• Changes have been tested locally
• Changes have been tested in testbeds
• Each commit in this PR is suitable for backport

[danth]

A diagram like in the description of #2434 could be useful to add.

I think it would help to know who to ask for reviews, and who has access to merge / close things.

It does not need to show everything, or be a strict process, just a quick guide would be nice.

flowchart TD
    A[Opened by author]
    B[Checked by module maintainer]
    C[Improved by author]
    D[Checked by core maintainer]
    E[Improved by author]
    F[Merged by core maintainer]
    G[Closed by core maintainer]
    A --> B
    B --->|Approved| D
    B -->|Changes requested| C
    C -->|Done| B
    C -->|Not done, after some time| G
    D --->|Approved| F
    D -->|Changes requested| E
    E -->|Done| D
    E -->|Not done, after some time| G

Loading

Even this is probably too complex.

[trueNAHO]

A diagram like in the description of #2434 could be useful to add.

I think it would help to know who to ask for reviews, and who has access to merge / close things.

It does not need to show everything, or be a strict process, just a quick guide would be nice.

flowchart TD
    A[Opened by author]
    B[Checked by module maintainer]
    C[Improved by author]
    D[Checked by core maintainer]
    E[Improved by author]
    F[Merged by core maintainer]
    G[Closed by core maintainer]
    A --> B
    B --->|Approved| D
    B -->|Changes requested| C
    C -->|Done| B
    C -->|Not done, after some time| G
    D --->|Approved| F
    D -->|Changes requested| E
    E -->|Done| D
    E -->|Not done, after some time| G

Loading

Even this is probably too complex.

I agree that establishing the optional and mandatory responsibilities of core and module maintainers in the review policy would be valuable, although the current version only addresses the rejection policy.

IMHO, state diagrams are very difficult and slow to read compared to prose. It is also very hard to make correct and intuitive state diagrams. For example, your diagram misses the optionality of module maintainer involvement and misses several cycles.

I suggest squashing the following into the previous patch:

diff --git a/doc/src/modules.md b/doc/src/modules.md
index 3297dde4..abc78f65 100644
--- a/doc/src/modules.md
+++ b/doc/src/modules.md
@@ -250,8 +250,8 @@ number of maintainers:
   }
   ```

-The main responsibility of module maintainers is to update and fix their
-modules.
+The main responsibility of module maintainers is updating and fixing their
+modules, and offering optional reviews on relevant PRs.

 > [!NOTE]
 > If this is the first time you're adding yourself as a maintainer in Stylix,
diff --git a/doc/src/review_policy.md b/doc/src/review_policy.md
index 006f28f6..c4ede101 100644
--- a/doc/src/review_policy.md
+++ b/doc/src/review_policy.md
@@ -3,6 +3,9 @@
 The review policy defines collaboration expectations between contributors and
 maintainers.

+Core maintainers have final merge authority, while module maintainers offer
+optional support.
+
 To preserve maintainer bandwidth and codebase quality, maintainers may freely
 close pull requests remaining excessively burdensome 30 days after an initial
 major objection.

Note how the second paragraph establishes the merge path and the third paragraph establishes the rejection path.

[danth]

It is also very hard to make correct and intuitive state diagrams. For example, your diagram misses the optionality of module maintainer involvement and misses several cycles.

Yes, I think for 3-4 states it is fine but anything more it gets hard to read.

+The main responsibility of module maintainers is updating and fixing their
+modules, and offering optional reviews on relevant PRs.

What about something like

Module maintainers are responsible for updating and fixing their modules,
and being a point of contact for reviews in their area. Maintainers are not
expected to be constantly available, but they may be removed after several
months of inactivity.

IMO the maintainer list is most useful to find out who to ask for a review. Explicitly saying "reviews are optional" might encourage people to join the list when they don't actually want to be contacted. Of course, nothing should be mandatory either.

[trueNAHO]

+The main responsibility of module maintainers is updating and fixing their
+modules, and offering optional reviews on relevant PRs.

What about something like

Module maintainers are responsible for updating and fixing their modules,
and being a point of contact for reviews in their area. Maintainers are not
expected to be constantly available, but they may be removed after several
months of inactivity.

IMO the maintainer list is most useful to find out who to ask for a review. Explicitly saying "reviews are optional" might encourage people to join the list when they don't actually want to be contacted. Of course, nothing should be mandatory either.

Good point. What about:

diff --git a/doc/src/modules.md b/doc/src/modules.md
index 3297dde4..a42a4786 100644
--- a/doc/src/modules.md
+++ b/doc/src/modules.md
@@ -250,8 +250,9 @@ number of maintainers:
   }
   ```

-The main responsibility of module maintainers is to update and fix their
-modules.
+Module maintainers are responsible for updating and fixing their modules, and
+may be consulted on relevant matters. Module maintainers are free to resign and
+may be removed after prolonged inactivity.

 > [!NOTE]
 > If this is the first time you're adding yourself as a maintainer in Stylix,

Being contacted or consulted does not necessarily imply responding, making the

Maintainers are not expected to be constantly available

clause somewhat self-evident. However, not responding repeatedly classifies as "prolonged inactivity".

[danth]

+Module maintainers are responsible for updating and fixing their modules, and
+may be consulted on relevant matters. Module maintainers are free to resign and
+may be removed after prolonged inactivity.

Yes, this sounds okay.

[0xda157]

LGTM