In this case, we clearly didn't assign a reviewer to the PR for 34 days since the PR was created so may be PMCs/committers should make regular rounds to assign reviewers to any new PRs that don't have a reviewer assigned ?
Also, may be we should encourage reviewers with high review load to unassign themselves from PRs that they won't be able to review in a reasonable time. Probably PMCs/committers should jump in and assign additional reviewers. Thanks, Cham On Sat, Jan 26, 2019 at 9:29 AM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > I agree, the problem is not on the guideline but more on the behavior. > > Regards > JB > > On 26/01/2019 16:37, Thomas Weise wrote: > > In this case the guidelines are not the issue, rather there may be a > > shortage of review attention. Considering Beam is one of the top > > projects by activity/commits, we may want to check why we have this > > problem. Is it perhaps that committers are too focused on their own PRs > > rather than review of non-committer contributions? Perhaps we should > > improve identifying and prioritizing PRs that come from new contributors > > or occasional contributors since that is needed to grow the community? > > The overall goal should probably be to balance the few full time, high > > frequency contributors/committers with the much larger potential pool of > > occasional contributors (some of which contribute in their spare time). > > > > Back to the guidelines: I currently don't see as much of a problem with > > the guidelines themselves, but with how they are applied.. I think that > > above mentioned high frequency committers should stick very close to the > > guidelines to make contributing to Beam more feasible to the community > > at large. I have seen few others comment on this as well, but recently > > it has been painful to rely on the master branch. Frequent build > > breakages or regressions make me want to sync less frequently, because I > > never know what surprise a pull might bring (time sink to resolve > issues). > > > > Thomas > > > > > > On Fri, Jan 25, 2019 at 10:34 PM Jean-Baptiste Onofré <j...@nanthrax.net > > <mailto:j...@nanthrax.net>> wrote: > > > > Agree. > > > > Unfortunately, I'm not so surprise about this feedback, even if no > > review at all is pretty rare. We improved a lot about the PR review, > but > > it's still too long IMHO. As I said several times, IMHO, if a PR is > > basically good and the build pass, it should be merged. > > We do a lot of discussion in PR, the build is not easy (sorry, but > > gradle just sucks at least on my box), we should give more chance to > > people to really participate (sometime, we just do things straight > > forward). > > > > I'm taking my hat as someone who started to lose motivation on Beam. > I'm > > forcing myself to come back more active on the project, but it's > hard: > > lot of messages/discussion, feeling that some parts can't be changed, > > that we are struggling and wasting time on non relevant discussion or > > already define by Apache policies, etc. > > > > So, at some degree, I understand such contributor feedback. > > > > Regards > > JB > > > > On 25/01/2019 20:03, Kenneth Knowles wrote: > > > Yea, that guide oscillates in size because of the tension between > (1) > > > being short enough that someone actually reads it and can get a > > > birds-eye view of what they have to do and (2) providing all the > info > > > needed in case they don't already know what to do. It is already > > pretty > > > short, and most of the technical details are off on the wiki. > > > > > > Kenn > > > > > > On Fri, Jan 25, 2019 at 10:21 AM Rui Wang <ruw...@google.com > > <mailto:ruw...@google.com> > > > <mailto:ruw...@google.com <mailto:ruw...@google.com>>> wrote: > > > > > > We have code contribution guidelines [1] and it says useful > > tips to > > > make PR reviewed and merged. But I guess it hides in Beam > > website so > > > new contributors are likely to ignore it. In order to make the > > > guidance easy to find and read for new contributors, we > > probably can > > > > > > a. Move number 5 item from [1] to a separate section and name > it > > > "Tips to get your PR reviewed and merged" > > > b. Put the link to the Github pull request template, so when a > > > contributor creates the first PR, the contributor could see > > the link > > > (or even paste text from contribution guide). It will be a good > > > chance that new contributors read what's in pull request > template. > > > > > > > > > -Rui > > > > > > [1] https://beam.apache.org/contribute/#make-your-change > > > > > > On Fri, Jan 25, 2019 at 9:24 AM Alexey Romanenko > > > <aromanenko....@gmail.com <mailto:aromanenko....@gmail.com> > > <mailto:aromanenko....@gmail.com <mailto:aromanenko....@gmail.com>>> > > wrote: > > > > > > For sure, it’s a pity that this PR has not been addressed > > for a > > > long time (I guess, we probably have other ones like this) > > but, > > > as I can see from this PR history, review has not been > > requested > > > explicitly by author (and this is one of the our > > recommendations > > > for code contribution [1]). > > > > > > What are the options to improve this: > > > > > > 1) Make it more clearly for new contributors that they > need to > > > ask for a review explicitly (with a help of > > recommendations that > > > already provided in top-right corner on PR page) > > > 2) Create a bot (like “stale” bot that we have) to check > for > > > non-addressed PRs that are more than, say, 7 days, and send > > > notification to dev@ (or dedicated, see n.3) mailing list > if > > > they are starving for review. > > > 3) (Optionally) Create new mailing list called pr@ for new > > > coming and non-addressed PRs > > > > > > [1] https://beam.apache.org/contribute/#make-your-change > > > > > > > > >> On 25 Jan 2019, at 17:50, Ismaël Mejía <ieme...@gmail.com > > <mailto:ieme...@gmail.com> > > >> <mailto:ieme...@gmail.com <mailto:ieme...@gmail.com>>> > wrote: > > >> > > >> The fact that this happened is a real pity. However it is > > >> clearly an > > >> exception and not the rule. Really few PRs have been long > > time > > >> without > > >> review. Can we somehow automatically send a notification > if a > > >> PR has > > >> no assigned reviewers, or if it has not been reviewed > after > > >> some time > > >> as Tim suggested? > > >> > > >> On Fri, Jan 25, 2019 at 9:43 AM Tim Robertson > > >> <timrobertson...@gmail.com > > <mailto:timrobertson...@gmail.com> <mailto:timrobertson...@gmail.com > > <mailto:timrobertson...@gmail.com>>> > > >> wrote: > > >>> > > >>> Thanks Kenn > > >>> > > >>> I tend to think that timing is the main contributing > factor > > >>> as you note on the Jira - it slipped down with no > > reminders / > > >>> bumps sent on any channels that I can see. > > >>> > > >>> Would something that alerts the dev@ list of PRs that > have > > >>> not received any attention after N days be helpful > perhaps? > > >>> Even if that only prompts action by one of us to comment > on > > >>> the PR that it's been acknowledged would likely be > enough to > > >>> engage the contributor - they would hopefully then ping > the > > >>> individual if it then slips for a long time. > > >>> > > >>> Next week will be my first I'll be able to work on Beam > in > > >>> 2019, but I'll comment on that PR now too as it's > > missing tests. > > >>> > > >>> > > >>> > > >>> > > >>> > > >>> On Fri, Jan 25, 2019 at 7:27 AM Kenneth Knowles > > >>> <k...@apache.org <mailto:k...@apache.org> > > <mailto:k...@apache.org <mailto:k...@apache.org>>> wrote: > > >>>> > > >>>> The subject line is a quote from BEAM-6324* > > >>>> > > >>>> This makes me sad. I hope/expect it is a failure to > route a > > >>>> pull request to the right reviewer. I am less sad about > the > > >>>> functionality than the sentiment and how a contributor > is > > >>>> being discouraged. > > >>>> > > >>>> Does anyone have ideas that could help? > > >>>> > > >>>> Kenn > > >>>> > > >>>> *https://issues.apache.org/jira/browse/BEAM-6324 > > > > > > > -- > > Jean-Baptiste Onofré > > jbono...@apache.org <mailto:jbono...@apache.org> > > http://blog.nanthrax.net > > Talend - http://www.talend.com > > > > -- > Jean-Baptiste Onofré > jbono...@apache.org > http://blog.nanthrax.net > Talend - http://www.talend.com >
