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> 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>> 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>> 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>> 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>> > >> 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>> 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 > http://blog.nanthrax.net > Talend - http://www.talend.com >
