Hello, I understand Julian's frustration. We all know that reviewing PRs is a recurring problem, and it is not the first time we discuss potential solutions, see e.g. the discussion a year ago [1] (also started by Julian) where several ideas were mentioned: automatic assignment, emulate the RM process onto the reviewing process (quite similar to the current proposal), ...but in the end no change was implemented, and the problem remains.
I agree that something must be done in order to revert the situation. In my opinion, one of the main factors is that the vast majority of PRs (even the ones that get merged) are never assigned. This lack of assignee can be seen as if the PR is "no-one's responsibility", so we should try somehow to assign each PR to someone, and make that person accountable for the PR's progression. I think we could try Julian's idea of having a pool of reviewers and a PR manager (taken from the pool, rotating this position every month or every two months). Personally, I would not set hard deadlines (e.g. something must be done within 3 days), because we are all volunteers and, even if we are all trying to do our best here, it may happen that a certain week we are busy with other personal or professional stuff. In the end, I think it should be the PR manager's responsibility to ping the assigned reviewer if a PR is not progressing after a reasonable period of time, ask them for an update, maybe even involve a different reviewer / re-assign the PR as a last resource. Of course, it must remain clear that, even if we implement this approach, anyone is still free (and encouraged) to participate in any PR review. Even if someone is not the assigned reviewer, they can chime in and contribute to the review nevertheless. Also, I think another sensible rule would be: if someone from the reviewers pool submits a PR, the PR manager will need to assign it to a different person. One last comment, I have the impression that with this initiative we would be moving towards a "better done than perfect" approach. Calcite is a vast project, with many different modules, and it could happen (it *will* happen) that a certain reviewer gets assigned a PR concerning a part of the project that they are not familiar with. Of course, one way of becoming (progressively) familiar with unknown parts of the project is by reviewing this type of PRs, but that takes time. I guess it would be possible for the assignee to try to ping and involve other reviewers with more experience in that area, but at the end of the day, it would be the assignee's responsibility to review and merge some piece of code that might be a bit obscure to them. This might lead to suboptimal or even incorrect code being inadvertently merged into the main branch. This is not a catastrophe (it can already happen with the current approach), and we will detect and correct these mistakes; I'm just mentioning that they might become a bit more frequent with the proposed approach (and we should all face them with a constructive and positive attitude). In any case, I have the impression that with the new idea the pros outweigh the cons, so we could give it a try. Best, Ruben [1] https://lists.apache.org/thread/30pf1o0vlcn7y3bhlcht1wdmvmxyvghn On Wed, Apr 12, 2023 at 3:13 AM Chunwei Lei <[email protected]> wrote: > Thanks Julian for sharing the proposal. I am +1 for it. I have been busy in > the past few months, so I have only had a quick look at the new JIRA. > However, I will have more time in the coming months, and I would be more > than happy to review any pull requests. > > > > Best, > Chunwei > > > On Tue, Apr 11, 2023 at 10:22 PM Jiajun Xie <[email protected]> > wrote: > > > Thank Julian for your idea. > > Your plan helps to motivate new contributors. > > > > “If there is no response to my PR, > > I will be disappointed or even give up on continuing to contribute.” > > > > I hope that every contributor will be encouraged, > > and I also hope that the Calcite community will become stronger and > > stronger. > > > > +1, I am willing to join the pool of reviews. > > > > On Tue, 11 Apr 2023 at 13:20, Benchao Li <[email protected]> wrote: > > > > > Thanks Julian for starting the discussion! > > > > > > I'm spending my spare time to contribute to Calcite, usually at > weekends, > > > and sometimes in the break of weekdays, hence my time would be limited > > > because the spare time may varies. Review work is not that simple for > me > > > because Calcite has many complicated components and evolves many years > > > which means we need track a lot of background. I'm still learning some > > part > > > while doing the review work. > > > > > > The complexity of PRs varies a lot, simple ones would be easier to get > in > > > because it only cost me minutes to hours to review. But the complex > ones, > > > usually I need to spend more time to understand the background, new > > design, > > > the effect to the whole project, and the future direction we want to > > take. > > > These kinds of PRs may be preempted by small ones, and finally do not > > > getting reviewed for months, there is a example[1] which I would say > > sorry > > > to the author that I still do not manage to give it a review till > today. > > > > > > Any way to improve current status would be grateful. However, if the > > > proposal from Julian may require more sustainable time, I'm not sure if > > it > > > is suitable for guys like me who only devotes limited spare time to > > > Calcite. Hence I'm +0 for this proposal. Of course, I would be happy to > > > participate in the schema if we finally decide to do it. > > > > > > [1] https://issues.apache.org/jira/browse/CALCITE-5413 > > > > > > Charles Givre <[email protected]> 于2023年4月11日周二 12:43写道： > > > > > > > I for one would very much like to help with reviews. I don't have a > > lot > > > > of time this month, but next month should have more time. > > > > Best, > > > > -- C > > > > > > > > > On Apr 10, 2023, at 10:56 PM, Dan Zou <[email protected]> wrote: > > > > > > > > > > +1, thanks Julian for proposing this. From my observation, there > are > > > > many pending PRs in Calcite and only a few active committers, this > > puts a > > > > lot of pressure on these committers. For example Julian have reviewed > > 34 > > > PR > > > > in 2023 Q1, it is an unimaginable number. I am very supportive of > > > achieving > > > > a mechanism to improve the review efficiency of PRs, and also I would > > > like > > > > to make contribution in reviewing PRs. > > > > > > > > > > Best, > > > > > Dan Zou > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >> 2023年4月11日 01:56，Julian Hyde <[email protected]> 写道： > > > > >> > > > > >> I don't enjoy reviewing and merging PRs. And every time I do, I > feel > > > > >> like a sucker, because there are over a few dozen committers who > are > > > > >> enjoying the project and not doing the work. (There is a small > group > > > > >> of committers who regularly review and merge PRs. I don't know how > > > > >> they feel about the task, but I am immensely grateful.) > > > > >> > > > > >> I think I would review more PRs if I saw others doing the same. > > > > >> > > > > >> Can we figure out a fairer way to distribute the load? For release > > > > >> managers (approximately the same amount of work, but compressed > > into a > > > > >> few hours or days) we have successfully run a rota for several > > years. > > > > >> Could we do something similar with PRs? > > > > >> > > > > >> I propose the following. For each calendar month, there is a PR > > > > >> manager and 6 - 8 reviewers. The PR manager does not review PRs, > but > > > > >> assigns them to reviewers, and politely reminds reviews to keep > the > > PR > > > > >> moving. > > > > >> > > > > >> The PR manager's goals are: > > > > >> * every non-draft PR is reviewed within 3 days of submission, > > > > >> * every PR is merged within 3 days of being done; > > > > >> * rotate duties so that no reviewer is asked to review more than 4 > > > > >> PRs per month; > > > > >> * email a report at the end of the month; > > > > >> * work down the backlog of historic PRs if it's a slow month. > > > > >> > > > > >> The PR manager rotates every month. The reviewers can rotate if > they > > > > >> wish, but I suspect most will stay in the pool for several months, > > > > >> because the reviewing load is not very heavy, and because they see > > > > >> others doing the work. > > > > >> > > > > >> Other notes: > > > > >> * Non-committers would be welcome to join the pool of reviews (and > > > > >> that would be a good way to earn the committer bit) and a > committer > > > > >> could merge when the PR is approved. > > > > >> * If committers join the pool, that's a good way to earn PMC > > > membership. > > > > >> * Committers who are not in the pool are welcome to review PRs and > > > > >> assign PRs to themselves (but expect to be nagged by the PR > manager > > if > > > > >> you don't review in a timely manner). > > > > >> > > > > >> What do you think? Would you join this scheme if we introduced it? > > If > > > > >> you agree please +1; also happy to see revisions to this > suggestion > > or > > > > >> other ideas to share the work. > > > > >> > > > > >> Julian > > > > > > > > > > > > > > > > > > > -- > > > > > > Best, > > > Benchao Li > > > > > >
