|
@@ -1,71 +1,67 @@
|
|
|
-# RFC: Update merge permissions for `o3de.org` repo
|
|
|
+# RFC: Require a docs reviewer and a technical-reviewer for PRs to `o3de.org`
|
|
|
|
|
|
-## :no_entry: If you submit a pull request to implement a process change without going through the RFC process, it may be closed with a polite request to submit an RFC first. :no_entry:
|
|
|
+Terminology:
|
|
|
+- docs reviewers: Refers to members of the `docs-reviewers` and `docs-maintainers` GitHub teams in the `o3de` GitHub organization. Both teams assume responsibility for reviewing documentation in `o3de.org` repo.
|
|
|
+- `docs-reviewers`: Specifically refers to the `docs-reviewers` team. [TODO LINK]
|
|
|
|
|
|
-### When using this template, you do not have to fill out every question below. The questions are there for guidance.
|
|
|
-
|
|
|
-This issue template should be used when filing an RFC related to _Documentation and Community processes and practice changes_ only. As a result, this RFC format should be used exclusively for discussions relating to SIG business which require extensive feedback from the community because it adds a new (or fundamentally changes an existing) process. Whether this process affects only the members of the SIG or all contributors, discussion is conducted in the open.
|
|
|
-
|
|
|
-This RFC template should be used for any suggestion that is not based upon code or content related to the O3DE product itself. This template is for proposing new process models, approaches, or ideas to improve the O3DE community.
|
|
|
-
|
|
|
-A hastily proposed RFC can hurt its chances of acceptance. Low-quality proposals, proposals for previously-rejected features, or those that don't fit into the near-term roadmap may be quickly rejected, demotivating the unprepared contributor. Laying some groundwork ahead of the RFC can make the process smoother.
|
|
|
-
|
|
|
-Although there is no single way to prepare for submitting an RFC, it is generally a good idea to pursue feedback from other developers beforehand. Talking to the core team members will help you prepare for a successful RFC.
|
|
|
-
|
|
|
-The most common preparations for writing and submitting an RFC include:
|
|
|
+## Summary
|
|
|
|
|
|
-* Talking the idea over on our Discord server.
|
|
|
-* Discussing the topic on our GitHub RFCs discussions page.
|
|
|
-* Occasionally posting "pre-RFCs" on the GitHub RFCs discussion page.
|
|
|
+This RFC proposes that submissions to the `o3de/o3de.org` repo, in the `/content/docs` and `/content/static/images` directories, require at least one docs reviewer and at least one technical reviewer to review and approve the PR before it can be merged.
|
|
|
|
|
|
-You may file issues in the RFCs repo for discussion, but these are not actively looked at by the teams.
|
|
|
+### What is the motivation for this suggestion?
|
|
|
|
|
|
-As a rule of thumb, receiving encouraging feedback from long-standing project developers, and particularly members of the relevant sub-team, is a good indication that the RFC is worth pursuing.
|
|
|
+*Why is this important?*
|
|
|
+By requiring a docs reviewer and a technical reviewer, incoming PRs to the `o3de.org`repo, in the `/content/docs` and `/content/static/images` directories, can ensure that the documentation is reviewed by the appropriate people. Furthermore, limiting the reviewers to one docs reviewer and one technical reviewer at minimum ensures efficient use of resources (reviewers).
|
|
|
|
|
|
-# ----- DELETE EVERYTHING FROM THE TOP TO THE SUMMARY LINE BELOW WHEN USING TEMPLATE ----- #
|
|
|
+*What are the use cases for this suggestion?*
|
|
|
+Any PR in the `o3de.org` repo that amends the `/content/docs/` or `/content/static/images` directory by updating existing docs or creating new docs and supplemental media, such as images and videos. For example, a PR that adds documentation about a feature.
|
|
|
|
|
|
-## Summary
|
|
|
+*What should the outcome be if this suggestion is implemented?*
|
|
|
+For a given PR, as described in the previous use case, a total of 2 approvals are required before the PR can be merged: one review from the `docs-reviewer` team and one review from the `technical-reviewer` team. A docs reviewer reviews the PR with a focus on how well it's written, and a technical reviewer focuses on the PR's technical accuracy.
|
|
|
|
|
|
-This RFC proposes that we change the merge permissions for the `main` branch in `o3de/o3de.org` repo, requiring that 1 docs-reviewer and 1 technical reviewer approves the PR before it can be merged.
|
|
|
|
|
|
-### What is the motivation for this suggestion?
|
|
|
+### Suggestion design description
|
|
|
|
|
|
-Incoming PRs to the `o3de.org` repo should be reviewed by the appropriate people.
|
|
|
+**1. A CODEOWNERS file**
|
|
|
+The `o3de.org:main` and `o3de.org:development` branches shall contain a CODEOWNERS file that dictates the `docs-reviewers` and `technical-reviewers` teams as owners of the appropriate directories in the `o3de.org` repo. This implementation is described and handled by the corresponding, accepted RFC, https://github.com/o3de/sig-docs-community/issues/61, and will help support the implementation of this proposed RFC.
|
|
|
|
|
|
-* Why is this important?
|
|
|
-* What are the use cases for this suggestion?
|
|
|
-* What should the outcome be if this suggestion is implemented?
|
|
|
+**2. Merge permissions that requires 1 docs reviewer and 1 technical reviewer**
|
|
|
+The merge permissions of the `o3de.org:main` and `o3de.org:development` branches shall be amended to require approvals from the designated owners that's set in the CODEOWNERS file. More information is needed on how to set up merge permissions to require one docs reviewer and one technical reviewer. However, the following describes one possible way:
|
|
|
+
|
|
|
+In GitHub's web interface, in `o3de.org` repo, you can amend the branch protection settings [TODO LINK] of the `main` and `development` branches by enabling **Require review from Code Owners**. When enabled, any PRs to these branches will require an approved review from the designated owners of the changed files. However, the CODEOWNERS file has a limitation: If multiple code owners are listed, the required approvals can be from any of those owners. For example, if the code owners are `docs-reviewers` and `technical-reviewers`, then a PR that recieves at least two approvals from docs reviewers and none from technical reviewers, or vice versa, will be valid to merge. This is not the functionality that this RFC proposed and a proof-of-concept to work around this limitation is needed.
|
|
|
|
|
|
-### Suggestion design description
|
|
|
+A way to work around this limitation is by defining merge configurations through a GitHub Action or App. One possible solution that's worth exploring is [Mergeable](https://github.com/mergeability/mergeable), a GitHub App that offers configurations to automate GitHub workflows. Specifically, it's ability to [configure approvals](https://mergeable.readthedocs.io/en/latest/validators/approval.html) can help implement this RFC. Mergeable's approval configuration offers the setting, `requested_reviewers`, which requires all of the requested reviewer's approvals in order for a PR to be merged. In the case of this RFC, a Mergeable configuration can specify that the required reviewers be the designated owners as defined in the CODEOWNERs file *and* that all the requested reviewers approve the PR.
|
|
|
|
|
|
-A few things need to be set up for this to take effect:
|
|
|
-1. We set up a CODEOWNERS file, as indicated by https://github.com/o3de/sig-docs-community/issues/61. The CODEOWNERS file that results from RFC#61 will affect this proposed RFC by setting up docs-reviewers and technical-reviewers group as owners of `/content/docs/*` directory.
|
|
|
-1. We change the merge permissions in `o3de.org` repo so that it requires requires approvals from the designated owners that's set in the CODEOWNERS file.
|
|
|
+**3. Technical reviewers**
|
|
|
+A plan to grow the `technical-reviewer` team shall be executed. This plan shall include a strategy to initially establish a sufficient team of `technical-reviewer` members -- a critical part of adopting this RFC because it ensures that there are enough reviewers to support an efficient PR process.
|
|
|
|
|
|
+The work to establish the `technical-reviewer` team may also include reassessing the current [Nomination Guidelines](https://github.com/o3de/sig-docs-community/blob/main/governance/reviewers-maintainers.md) for technical reviewers.
|
|
|
|
|
|
### What are the advantages of the suggestion?
|
|
|
|
|
|
-* Explain the advantages of using this suggestion.
|
|
|
+- Automatically ensures a PR receives adequete reviewer approvals before it can be merged.
|
|
|
+- Helps improve the quality of the documentation, including how accurate the technical information is and how well it's written.
|
|
|
+- Clarifies responsibilities in the PR process for `o3de.org` repo.
|
|
|
|
|
|
### What are the disadvantages of the suggestion?
|
|
|
|
|
|
-- Explain any disadvantages of this change. Please take counterarguments in good faith and accurately represent any concerns that you or others have shared about the suggestion.
|
|
|
|
|
|
### How will this be work within the O3DE project?
|
|
|
|
|
|
-- Explain how this suggestion will be work within the O3DE project.
|
|
|
+This will be implemented in the PR process in `o3de.org` repo.
|
|
|
|
|
|
### Scope of work
|
|
|
|
|
|
Include an estimation of the scope of work, so the SIG can determine if it can be reduced to a single issue (or small set of issues) an individual can perform, or if it needs to be a coordinated effort as an official SIG project.
|
|
|
|
|
|
+There are two main tasks for this RFC:
|
|
|
+- The CODEOWNERS file and the `o3de.org:main` and `o3de.org:development` branches must be set up to enforce required reviews from 1 docs reviewer and 1 technical reviewer. For a detailed description of the implementation, see **Suggestion design description**.
|
|
|
+- Initializing the `technical-reviewer` team so that it contains the appropriate members needed to review PRs to `o3de.org` repo. The starting number of members must also be enough to ensure an efficient PR process.
|
|
|
+
|
|
|
### Are there any alternatives to this suggestion?
|
|
|
|
|
|
-- Provide any other alternative ways that have been considered.
|
|
|
-- Explain what the impact might be of not implementing this suggestion.
|
|
|
-- If there are other similar suggestions previously used, list them and explain which parts may have solved some or all of this problem.
|
|
|
+An alternative to the suggestion proposed in this RFC is to keep the review requirement rules as they currently are. The impact of this would result in repeated issues that have occasionally occurred in the PR process for `o3de.org` repo. Issues include merged PRs that amend technical docs and did not receive an approval from an appropriate technical reviewer, or in other cases, did not receive an approval from a docs reviewer. A lack of structure and unclear roles in the PR process is not scalable for both the quality of O3DE Documentation and for the growing community of contributors to O3DE.
|
|
|
|
|
|
### What is the strategy for adoption?
|
|
|
|
|
|
-- Point out any efforts needed if it requires coordination with multiple SIGs or other projects.
|
|
|
-- Explain how it would be adopted by new and existing contributors.
|
|
|
+If approved, sig-docs-community can explore, test, and implement a solution that enforces the proposed required reviewers for the `o3de.org` repo. With coordination from other SIGs, the `technical-reviewer` team can be populated with appropriate members.
|