For a long time this has been one of the main issues of the project and I am happy to see discussions to address this issue.
I would like to mention that as a contributor, I am, and always have been very grateful to people reviewing my work. The fact that I became a committer of this project is mainly due to Julian and Vladimir Sitnikov who reviewed and merged many of my PRs. I would definitely like to help and make other contributors feel the same but I cannot really commit to specific volume and deadlines spanning several months. I have the feeling that we don't need the PR manager role. The assignment work can be done by bots (e.g., [1]) if needed and we already have our quarterly stats for reporting purposes. If we want to put a human behind this then it makes more sense for this person to be the release manager; this should be the one nagging people for advancing the work and moving towards the release. Regarding reviews coming from non-committers, I am not sure it's possible to do the assignment in GitHub. It's not a big deal though; for me a simple comment that I am going to review would be sufficient. Alternatively, we could consider adopting an equivalent workflow in JIRA and potentially introducing a new state "IN REVIEW"; don't think it is necessary. No matter the choice we should ensure that we have a trackable way to recognise "non-committer" reviewers but I think both GitHub (e.g., "is:pr reviewed-by:julianhyde is:closed") and JIRA offer the necessary filters; Others projects tend to include such info in the commit message we could also opt for this if we deem necessary. As an immediate action I would encourage everyone willing to help to go to the open PRs on GitHub and either assign some PRs to themselves (in case of committers) or leave a comment about their intention (non-committers). In the meantime we can iterate on this till we reach consensus. Best, Stamatis [1] https://github.com/apps/auto-assign On Wed, Apr 12, 2023 at 10:49 AM Ruben Q L <rube...@gmail.com> wrote: > > 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 <chunwei.l...@gmail.com> 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 <jiajunbernou...@gmail.com> > > 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 <libenc...@apache.org> 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 <cgi...@gmail.com> 于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 <zoud...@163.com> 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 <jh...@apache.org> 写道： > > > > > >> > > > > > >> 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 > > > > > > > > >
