[GitHub] [tvm] electriclilies edited a comment on issue #9057: [VOTE] Adopt round-robin assignment of reviewers for GitHub pull request reviewer assignment.

[GitBox <gi...@apache.org>]

electriclilies edited a comment on issue #9057:
URL: https://github.com/apache/tvm/issues/9057#issuecomment-930626090


   Thanks @manupa-arm @Mousius and @leandron for bringing these points up. 
   
   
Building off what you said, I just wanted to point out that I think that this vote is really two questions rolled into one:
   **Q1:** Whether the current system of tagging all code owners is working, and should we revert it
   **Q2:** Whether we want to move to a system where we are tagging only 2 of the code owners for round-robin review
   
   From what I’ve observed, tagging all code owners is not working. Right now, there’s a small subset of committers who do a large amount of shepherding code. These people are the ones whose workflows have been most affected because they are being tagged in nearly everything and can’t sort through the noise. I think that this is a good reason to revert the system where we tag all code owners in the short term. 

However, I don’t think that moving to the round robin review where we tag 2 people out of the current code owners list is a good long-term solution. Most of the code owners are not active so the result will pretty much be going back to the way things are where we just tag code owners.
   
   Backing up a bit, I think the three main goals of tagging all code owners was:
   **G1.** Spread the load of “close reviewing” to more committers
   **G2.** Increase visibility across TVM subprojects (i.e., we might want someone who works on automation to look at microTVM PRs (and vice-versa) to make sure that the goals of those subproject are consistent)
   **G3.** Give new community members who don’t have a network inside TVM visibility
   
   Tagging all code owners did not accomplish G1, but I think it is helpful in getting us closer to G2 and G3. (Although it is worth noting that I only think it helps us get closer to G2 since it tags so many people rather indiscriminately). If I understand correctly, Manupa, Christopher and Leandro’s concern is that the round robin solution does not accomplish G1 and moves us further from accomplishing G2 and G3. 
   
   I think that (G1) and (G2 and G3) require separate but complimentary solutions. Before talking about those solutions, though, it’s helpful to consider the kinds of reviews that there are in TVM and their purpose:
   
   **R1.** Review from someone who works closely on this aspect of the project and can provide very specific feedback based on a “close reading”
   (Purpose of R1: Ensuring good-quality code and getting it in in time (code shepherding))
   
   **R2.** Review from someone who is not familiar with this subproject but is doing a “close reading” with the goal of that person gaining domain knowledge of that subproject
   (Purpose of R2: Knowledge spreading; creating more people who can do “close readings” of this subproject (creating more code shepherds))
   
   **R3.** Review from someone with expertise in another part of the project for consistency with overall TVM goals and cross-company / cross-subproject networking
   (Purpose of R3: Ensuring project consistency and preventing siloing of groups of contributors)
   
   To spread the reviewing load out (G1), we need close reviews from more code shepherds (R1). I think that the easiest way to do this is to **drastically increase the number of committers**. To do this, we should make it a lot easier to become a committer. Specifically, I think we should consider allowing committers to nominate reviewers. Additionally, we should consider having a bot that notifies the community when contributors and reviewers hit certain milestones (like number of PRs submitted, LOC submitted, etc). It would still be up to a committer / PMC member to do the nomination, but this would essentially create a short-list of people to nominate. Another benefit of this is that it will reduce cognitive load on PMC and committers because they don’t need to be watching for who to nominate. 
   
   Additionally, for community members to gain domain knowledge, they need to be able to do R2s, which means they need visibility into PRs relevant to a certain area. From this standpoint, I think that fine-tuning the code owners of a certain part of the codebase and then notifying all of those code owners is the way to go. 
(Also, I think that“code owners” is not the right concept here— ideally the people notified would be everyone who wants to be notified of PRs in that part of the codebase, whether they are contributor, reviewer or committer. This way everyone gets notified for what they want to be notified. As a reviewer I am not pinged as a code owner, even if I would like to be). 
   
   To increase R3s, it would be nice to do a round-robin where we randomly tag people who are not “code owners” for the part of the project the PR is relevant to (in addition to tagging all the code owners). Ideally we’d limit the number of times someone gets tagged for an R3 review to once or twice a month so it’s not too much and they can adequately engage in the conversation if they want to.
   
   Finally, I just wanted to clarify that I don't think reviewing people's code should be mandatory in any way. (But like @leandron said it would be helpful to have a way to communicate to the community if you are unavailable). And, I think that auto-tagging people is supplementary to contributors tagging other contributors -- we still want people to have working relationships with other contributors). 
   
   In summary, I think we should:
   1. Increase the number of committers by making it easier to become a reviewer and creating a bot that reports contributor milestones.
   2. Fine-tune who is notified for parts of the project via some sort of opt-in / opt-out system and notify all of them for PRs in that part of the project. Additionally, we should allow people who are reviewers and contributors to opt-in so that they can keep up with what is going on.
   3. Occasionally notify people on PRs for subprojects they don’t regularly work in to facilitate cross pollination and consistency across the project.
   
   I’m not saying we should implement all of this right now, but I'd like to see us moving towards a world like this (though my thoughts are by no means finalized, so if you have any suggestions / feedback / thoughts please share them!)
   cc @denise-k @hogepodge Also interested to see what you guys think!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org