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 we tagged 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). 

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!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm/issues/9057#issuecomment-930626090

Reply via email to