RFC: Establishing a Pull Request Review and Merge Process

[ivangarcia44]

This RFC proposes a new process for reviewing and merging pull requests in the torch-mlir project to ensure timely progress for all contributors and that feedback is properly addressed. Currently PR’s usually take weeks to merge which can slow down development. There is also a significant amount of open pull requests (inventory "muda") which could lead to risky merges, lost features, potential bugs and duplicate work The proposed guidelines aim to balance thorough review with development velocity.

Main motivations: • Clarify expectations for both contributors and reviewers. • Ensure all stakeholders have sufficient opportunity to provide feedback without blocking progress indefinitely. • Encourage broader participation and make the project more attractive to new contributors. • Ensure that all feedback is properly addressed. • Reduce the pull request inventory muda (see seven wastes of the “Lean Manufacturing” method from Toyota Production System: https://en.wikipedia.org/wiki/Lean_manufacturing).

@stellaraccident, @sjarus @vivekkhandelwal1 @tanyokwok @qiuxiafei @ZihengJiang @Vremold

Proposal

Add a page under the "Contributing" section of https://github.com/llvm/torch-mlir/wiki with title: "Pull Request Review and Merge Process".

The wiki page linked there will have the contents below. Please let me know if it is ok to add this page and if any modifications should be done to it.

---------------------------------------------- Start of wiki page contents -------------------------------------------------

The torch-mlir project project contributors should follow the following steps to do pull requests reviews and merges.

1. Initial PR submission: The contributor submits a pull request and adds reviewers with ownership levels 1 to 3 (see ownership section below) of the modified files.

2. One-week reminder: If no feedback has been received after one week, the contributor will post a reminder comment, tagging additional reviewers with ownership of levels 4 to 6.

3. Two-week reminder: If there is still no feedback after two weeks, then the contributor can add reviewers of ownership level 7.

4. Three-week merge window: After three weeks the contributor can merge the changes if at least one feedback is received and addressed from a reviewer of any ownership level.

5. Post-merge feedback: If feedback or concerns are raised after the merge, the contributor will create a follow-up pull request to address them.

6. Reviewer availability: Where possible, unavailable reviewers should comment on the pull request to notify contributors and recommend alternate reviewers. It is helpful that reviewers set their GitHub status to “busy” when they are not available (e.g., vacations, busy with a project).

7. Reviewers’ responsibility: Reviewers should prioritize on cleaning old pull requests, either by closing them or reviewing them for merging.

Ownership levels

1. Component owner as indicated in this page: https://github.com/llvm/torch-mlir/blob/main/docs/code_owners.md.
2. Contributor with highest number of modifications to the file modified in the pull request.
3. Contributor who modified the lines of code included in the file modified in the pull request.
4. Contributor who created the file modified in the pull request.
5. Contributor who has done any modifications to the modified file in the pull request.
6. Contributor who has modified a file in the same component (e.g., Linear.cpp is on the pull request, and contributor has modified a file in torch to linalg lowering).
7. Contributor to any file in torch-mlir

The author of the pull request must create a separate pull request to address feedback received after the merge.

----------------------------------------------- Endof wiki page contents --------------------------------------------------

Thank you, Ivan

[stellaraccident]

I'll admit that I have been just responding to PRs and issues that mention me by name for the last several months.

Looking through the backlog, we have a mix of PRs that are getting timely attention, those that got feedback but no author follow-up and a fair number that are going untended.

Also looking through, there are a fair number of people listed who I know are no longer active.

Before we just land a detailed policy, maybe we could take the time to poll who is active, update code owner docs as appropriate and generally touch base with people. Many people have moved companies and projects over the last two years, and it has been a long time since anyone has touched base.

Give me a day or two, but I will start a dedicated discussion thread with the people I can see/know to be active generally and get us talking to each other again. That will give us more information to reset structure and expectations.

Thanks for speaking up. There is a real problem here to solve.