You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Kenneth Knowles <ke...@apache.org> on 2019/01/25 06:26:56 UTC

BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

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

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Scott Wegner <sw...@google.com>.
For first-time contributors, GitHub will display a link to a contributor
guide [1]. We have one for Beam [2], which simply directs contributors to
the website contribution guide [3].

I think what we have is pretty good already, although we could be more
clear that the contributor guide includes tips like how to find a reviewer.
Here's an attempt at improving it: https://github.com/apache/beam/pull/7628

[1]
https://help.github.com/articles/setting-guidelines-for-repository-contributors/
[2] https://github.com/apache/beam/blob/master/CONTRIBUTING.md
[3] https://beam.apache.org/contribute/contribution-guide/

On Fri, Jan 25, 2019 at 10:21 AM Rui Wang <ru...@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 <ar...@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 <ie...@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 <ti...@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 <ke...@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
>>
>>
>>

-- 




Got feedback? tinyurl.com/swegner-feedback

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Chamikara Jayalath <ch...@google.com>.
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é <jb...@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é <jb@nanthrax.net
> > <ma...@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 <ruwang@google.com
> >     <ma...@google.com>
> >     > <mailto:ruwang@google.com <ma...@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.dev@gmail.com <ma...@gmail.com>
> >     <mailto:aromanenko.dev@gmail.com <ma...@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 <iemejia@gmail.com
> >     <ma...@gmail.com>
> >     >>         <mailto:iemejia@gmail.com <ma...@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
> >     >>         <timrobertson100@gmail.com
> >     <ma...@gmail.com> <mailto:timrobertson100@gmail.com
> >     <ma...@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
> >     >>>         <kenn@apache.org <ma...@apache.org>
> >     <mailto:kenn@apache.org <ma...@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é
> >     jbonofre@apache.org <ma...@apache.org>
> >     http://blog.nanthrax.net
> >     Talend - http://www.talend.com
> >
>
> --
> Jean-Baptiste Onofré
> jbonofre@apache.org
> http://blog.nanthrax.net
> Talend - http://www.talend.com
>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
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é <jb@nanthrax.net
> <ma...@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 <ruwang@google.com
>     <ma...@google.com>
>     > <mailto:ruwang@google.com <ma...@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.dev@gmail.com <ma...@gmail.com>
>     <mailto:aromanenko.dev@gmail.com <ma...@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 <iemejia@gmail.com
>     <ma...@gmail.com>
>     >>         <mailto:iemejia@gmail.com <ma...@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
>     >>         <timrobertson100@gmail.com
>     <ma...@gmail.com> <mailto:timrobertson100@gmail.com
>     <ma...@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
>     >>>         <kenn@apache.org <ma...@apache.org>
>     <mailto:kenn@apache.org <ma...@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é
>     jbonofre@apache.org <ma...@apache.org>
>     http://blog.nanthrax.net
>     Talend - http://www.talend.com
> 

-- 
Jean-Baptiste Onofré
jbonofre@apache.org
http://blog.nanthrax.net
Talend - http://www.talend.com

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Thomas Weise <th...@apache.org>.
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é <jb...@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 <ruwang@google.com
> > <ma...@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.dev@gmail.com <ma...@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 <iemejia@gmail.com
> >>         <ma...@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
> >>         <timrobertson100@gmail.com <ma...@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
> >>>         <kenn@apache.org <ma...@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é
> jbonofre@apache.org
> http://blog.nanthrax.net
> Talend - http://www.talend.com
>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
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 <ruwang@google.com
> <ma...@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.dev@gmail.com <ma...@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 <iemejia@gmail.com
>>         <ma...@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
>>         <timrobertson100@gmail.com <ma...@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
>>>         <kenn@apache.org <ma...@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é
jbonofre@apache.org
http://blog.nanthrax.net
Talend - http://www.talend.com

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Kenneth Knowles <kl...@google.com>.
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 <ru...@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 <ar...@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 <ie...@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 <ti...@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 <ke...@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
>>
>>
>>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Etienne Chauchot <ec...@apache.org>.
I also missed the sentence Kenn mentioned. I think it is worth enlightening it.
Thx for your PR around that Lukasz !
Etienne
Le mercredi 30 janvier 2019 à 11:03 +0100, Łukasz Gajowy a écrit :
> Wow. I missed the sentence. Judging from the fact that others also proposed adding it, I think it might need some
> care. I proposed a PR here: https://github.com/apache/beam/pull/7670
> 
> Łukasz
> śr., 30 sty 2019 o 00:39 Kenneth Knowles <kl...@google.com> napisał(a):
> > On Mon, Jan 28, 2019 at 5:25 AM Łukasz Gajowy <lg...@apache.org> wrote:
> > > IMHO, I don't think committers spend time watching new PRs coming up, but they more likely act when pinged. So, we
> > > may need some automation in case a contributor do not use github reviewed proposal. Auto reviewer assignment seem
> > > too much but modifying the PR template to add a sentence such as "please pickup a reviewer in the proposed list"
> > > could be enough. 
> > > WDYT ?
> > > 
> > > and
> > > 
> > > (1) A sentence in the PR template suggesting adding a reviewer. (easy)
> > > 
> > > 
> > > +100! Let's improve the message in the PR template. It costs nothing and can help a lot. I'd say it should be in
> > > bold letters as this is super important.
> > > 
> > 
> > There is already a message. Is it unclear? Can you rephrase it to something better?
> > Kenn 
> > > Maybe this is also worth reconsidering if auto reviewer assignment (or at least some form of it) is a bad idea. We
> > > can assign committers (the most "hardcore" option, maybe too much) or ping them in emails/github comments if
> > > there's inactivity in pull requests (the soft one but requires a bot to be implemented). The way I see this is
> > > that such auto assigned reviewer could always say "I have lots on my plate" but suggest someone else to take care
> > > of the PR. This way the problem that nobody is mentioned by the PR author is completely gone. Other than that,
> > > such an approach feels efficient to me because it's more "in person" (similar to what Robert said). 
> > > 
> > > It's certainly disheartening as a
> > > reviewer to put time into reviewing a PR and then the author doesn't
> > > bother to even respond, or (as has happened to me) be told "hey, this
> > > wasn't ready for review yet."
> > > 
> > > As for "this wasn't ready for review" - there are sometimes situations that require a PR to be opened before they
> > > are actually completed (especially when working with Jenkins jobs). Given that there might be misunderstandings
> > > authors of such commits should give a clear message saying "do not merge yet" or "not ready for review" in title
> > > or comments or even close such PR and reopen until the change is ready. It's all about giving a clear signal to
> > > others. 
> > > 
> > > Maybe we should mention it in guidelines/PR message too to avoid situations like this?
> > > 
> > > Łukasz
> > > 
> > > 
> > > 
> > > pon., 28 sty 2019 o 11:30 Robert Bradshaw <ro...@google.com> napisał(a):
> > > > On Mon, Jan 28, 2019 at 10:37 AM Etienne Chauchot <ec...@apache.org> wrote:
> > > > >
> > > > > Sure it's a pity than this PR got unnoticed and I think it is a combination of factors (PR date around
> > > > Christmas, the fact that the author forgot - AFAIK - to ping a reviewer in either the PR or the ML).
> > > > >
> > > > > I agree with Rui's proposal to enhance visibility of the "how to get a reviewed" process.
> > > > >
> > > > > IMHO, I don't think committers spend time watching new PRs coming up, but they more likely act when pinged.
> > > > So, we may need some automation in case a contributor do not use github reviewed proposal. Auto reviewer
> > > > assignment seem too much but modifying the PR template to add a sentence such as "please pickup a reviewer in
> > > > the proposed list" could be enough.
> > > > > WDYT ?
> > > > 
> > > > 
> > > > +1
> > > > 
> > > > 
> > > > I see two somewhat separable areas of improvement:
> > > > 
> > > > 
> > > > (1) Getting a reviewer assigned to a PR, and
> > > > (2) Expectations of feedback/timeliness from a reviewer once it has
> > > > been assigned.
> > > > 
> > > > 
> > > > The first is the motivation for this thread, but I think we're
> > > > suffering on the second as well.
> > > > 
> > > > 
> > > > Given the reactions here, it sounds like most of us are just as
> > > > unhappy this happened as the author, and would be happy to pitch in
> > > > and improve things.
> > > > 
> > > > 
> > > > I agree with Kenn that just adding to more the contributor guide
> > > > always help because a contributor guide with everything one might need
> > > > to know is the least likely to actually be read in its entirety.
> > > > Rather it's useful to provide such guidance at the point that it's
> > > > needed. Specifically, I would like to see
> > > > 
> > > > 
> > > > (1) A sentence in the PR template suggesting adding a reviewer. (easy)
> > > > (2) An automated recommendation for suggesting good candidate
> > > > reviewers (if we deem Github's suggestions insufficient). (harder)
> > > > (3) A bot that follows up on PRs after 1 week(?) noting the lack of
> > > > action and suggesting (and, implicitly but importantly
> > > > permission/expectation) that the author bring the PR to the list.
> > > > (medium)
> > > > 
> > > > 
> > > > We could also have automated emails like the Beam Dependency Check
> > > > Report, but automated emails are much easier to ignore than personal
> > > > ones. Having the author ping dev@ has the added advantage that it
> > > > gives the author something they can do to move the PR forward, and
> > > > provides a clear signal that this is a PR someone cares enough about
> > > > to prioritize it above others. (It's certainly disheartening as a
> > > > reviewer to put time into reviewing a PR and then the author doesn't
> > > > bother to even respond, or (as has happened to me) be told "hey, this
> > > > wasn't ready for review yet." On the other hand it's rewarding to help
> > > > someone, especially a first time contributor, to see their change
> > > > actually get in. Improving this ratio will I think both increase the
> > > > productivity of reviews and the motivation to do them.)
> > > > 
> > > > 
> > > > > Also, I started to review the PR on Friday (thx Kenn for pinging me).
> > > > >
> > > > > Etienne
> > > > >
> > > > > Le vendredi 25 janvier 2019 à 10:21 -0800, Rui Wang a écrit :
> > > > >
> > > > > 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 <ar...@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 <ie...@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 <ti...@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 <ke...@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
> > > > >
> > > > >
> > > > 

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Łukasz Gajowy <lg...@apache.org>.
Wow. I missed the sentence. Judging from the fact that others also proposed
adding it, I think it might need some care. I proposed a PR here:
https://github.com/apache/beam/pull/7670

Łukasz

śr., 30 sty 2019 o 00:39 Kenneth Knowles <kl...@google.com> napisał(a):

>
>
> On Mon, Jan 28, 2019 at 5:25 AM Łukasz Gajowy <lg...@apache.org> wrote:
>
>> IMHO, I don't think committers spend time watching new PRs coming up, but
>> they more likely act when pinged. So, we may need some automation in case a
>> contributor do not use github reviewed proposal. Auto reviewer assignment
>> seem too much but modifying the PR template to add a sentence such as
>> "please pickup a reviewer in the proposed list" could be enough.
>> WDYT ?
>>
>> and
>>
>> (1) A sentence in the PR template suggesting adding a reviewer. (easy)
>>
>>
>> +100! Let's improve the message in the PR template. It costs nothing and
>> can help a lot. I'd say it should be in bold letters as this is super
>> important.
>>
>
> There is already a message. Is it unclear? Can you rephrase it to
> something better?
>
> Kenn
>
>
>
>> Maybe this is also worth reconsidering if auto reviewer assignment (or at
>> least some form of it) is a bad idea. We can assign committers (the most
>> "hardcore" option, maybe too much) or ping them in emails/github comments
>> if there's inactivity in pull requests (the soft one but requires a bot to
>> be implemented). The way I see this is that such auto assigned reviewer
>> could always say "I have lots on my plate" but suggest someone else to take
>> care of the PR. This way the problem that nobody is mentioned by the PR
>> author is completely gone. Other than that, such an approach feels
>> efficient to me because it's more "in person" (similar to what Robert
>> said).
>>
>> It's certainly disheartening as a
>> reviewer to put time into reviewing a PR and then the author doesn't
>> bother to even respond, or (as has happened to me) be told "hey, this
>> wasn't ready for review yet."
>>
>> As for "this wasn't ready for review" - there are sometimes situations
>> that require a PR to be opened before they are actually completed
>> (especially when working with Jenkins jobs). Given that there might be
>> misunderstandings authors of such commits should give a clear message
>> saying "do not merge yet" or "not ready for review" in title or comments or
>> even close such PR and reopen until the change is ready. It's all about
>> giving a clear signal to others.
>>
>> Maybe we should mention it in guidelines/PR message too to avoid
>> situations like this?
>>
>> Łukasz
>>
>>
>> pon., 28 sty 2019 o 11:30 Robert Bradshaw <ro...@google.com>
>> napisał(a):
>>
>>> On Mon, Jan 28, 2019 at 10:37 AM Etienne Chauchot <ec...@apache.org>
>>> wrote:
>>> >
>>> > Sure it's a pity than this PR got unnoticed and I think it is a
>>> combination of factors (PR date around Christmas, the fact that the author
>>> forgot - AFAIK - to ping a reviewer in either the PR or the ML).
>>> >
>>> > I agree with Rui's proposal to enhance visibility of the "how to get a
>>> reviewed" process.
>>> >
>>> > IMHO, I don't think committers spend time watching new PRs coming up,
>>> but they more likely act when pinged. So, we may need some automation in
>>> case a contributor do not use github reviewed proposal. Auto reviewer
>>> assignment seem too much but modifying the PR template to add a sentence
>>> such as "please pickup a reviewer in the proposed list" could be enough.
>>> > WDYT ?
>>>
>>> +1
>>>
>>> I see two somewhat separable areas of improvement:
>>>
>>> (1) Getting a reviewer assigned to a PR, and
>>> (2) Expectations of feedback/timeliness from a reviewer once it has
>>> been assigned.
>>>
>>> The first is the motivation for this thread, but I think we're
>>> suffering on the second as well.
>>>
>>> Given the reactions here, it sounds like most of us are just as
>>> unhappy this happened as the author, and would be happy to pitch in
>>> and improve things.
>>>
>>> I agree with Kenn that just adding to more the contributor guide
>>> always help because a contributor guide with everything one might need
>>> to know is the least likely to actually be read in its entirety.
>>> Rather it's useful to provide such guidance at the point that it's
>>> needed. Specifically, I would like to see
>>>
>>> (1) A sentence in the PR template suggesting adding a reviewer. (easy)
>>> (2) An automated recommendation for suggesting good candidate
>>> reviewers (if we deem Github's suggestions insufficient). (harder)
>>> (3) A bot that follows up on PRs after 1 week(?) noting the lack of
>>> action and suggesting (and, implicitly but importantly
>>> permission/expectation) that the author bring the PR to the list.
>>> (medium)
>>>
>>> We could also have automated emails like the Beam Dependency Check
>>> Report, but automated emails are much easier to ignore than personal
>>> ones. Having the author ping dev@ has the added advantage that it
>>> gives the author something they can do to move the PR forward, and
>>> provides a clear signal that this is a PR someone cares enough about
>>> to prioritize it above others. (It's certainly disheartening as a
>>> reviewer to put time into reviewing a PR and then the author doesn't
>>> bother to even respond, or (as has happened to me) be told "hey, this
>>> wasn't ready for review yet." On the other hand it's rewarding to help
>>> someone, especially a first time contributor, to see their change
>>> actually get in. Improving this ratio will I think both increase the
>>> productivity of reviews and the motivation to do them.)
>>>
>>> > Also, I started to review the PR on Friday (thx Kenn for pinging me).
>>> >
>>> > Etienne
>>> >
>>> > Le vendredi 25 janvier 2019 à 10:21 -0800, Rui Wang a écrit :
>>> >
>>> > 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.dev@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 <ie...@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 <
>>> timrobertson100@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 <ke...@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
>>> >
>>> >
>>>
>>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Kenneth Knowles <kl...@google.com>.
On Mon, Jan 28, 2019 at 5:25 AM Łukasz Gajowy <lg...@apache.org> wrote:

> IMHO, I don't think committers spend time watching new PRs coming up, but
> they more likely act when pinged. So, we may need some automation in case a
> contributor do not use github reviewed proposal. Auto reviewer assignment
> seem too much but modifying the PR template to add a sentence such as
> "please pickup a reviewer in the proposed list" could be enough.
> WDYT ?
>
> and
>
> (1) A sentence in the PR template suggesting adding a reviewer. (easy)
>
>
> +100! Let's improve the message in the PR template. It costs nothing and
> can help a lot. I'd say it should be in bold letters as this is super
> important.
>

There is already a message. Is it unclear? Can you rephrase it to something
better?

Kenn



> Maybe this is also worth reconsidering if auto reviewer assignment (or at
> least some form of it) is a bad idea. We can assign committers (the most
> "hardcore" option, maybe too much) or ping them in emails/github comments
> if there's inactivity in pull requests (the soft one but requires a bot to
> be implemented). The way I see this is that such auto assigned reviewer
> could always say "I have lots on my plate" but suggest someone else to take
> care of the PR. This way the problem that nobody is mentioned by the PR
> author is completely gone. Other than that, such an approach feels
> efficient to me because it's more "in person" (similar to what Robert
> said).
>
> It's certainly disheartening as a
> reviewer to put time into reviewing a PR and then the author doesn't
> bother to even respond, or (as has happened to me) be told "hey, this
> wasn't ready for review yet."
>
> As for "this wasn't ready for review" - there are sometimes situations
> that require a PR to be opened before they are actually completed
> (especially when working with Jenkins jobs). Given that there might be
> misunderstandings authors of such commits should give a clear message
> saying "do not merge yet" or "not ready for review" in title or comments or
> even close such PR and reopen until the change is ready. It's all about
> giving a clear signal to others.
>
> Maybe we should mention it in guidelines/PR message too to avoid
> situations like this?
>
> Łukasz
>
>
> pon., 28 sty 2019 o 11:30 Robert Bradshaw <ro...@google.com>
> napisał(a):
>
>> On Mon, Jan 28, 2019 at 10:37 AM Etienne Chauchot <ec...@apache.org>
>> wrote:
>> >
>> > Sure it's a pity than this PR got unnoticed and I think it is a
>> combination of factors (PR date around Christmas, the fact that the author
>> forgot - AFAIK - to ping a reviewer in either the PR or the ML).
>> >
>> > I agree with Rui's proposal to enhance visibility of the "how to get a
>> reviewed" process.
>> >
>> > IMHO, I don't think committers spend time watching new PRs coming up,
>> but they more likely act when pinged. So, we may need some automation in
>> case a contributor do not use github reviewed proposal. Auto reviewer
>> assignment seem too much but modifying the PR template to add a sentence
>> such as "please pickup a reviewer in the proposed list" could be enough.
>> > WDYT ?
>>
>> +1
>>
>> I see two somewhat separable areas of improvement:
>>
>> (1) Getting a reviewer assigned to a PR, and
>> (2) Expectations of feedback/timeliness from a reviewer once it has
>> been assigned.
>>
>> The first is the motivation for this thread, but I think we're
>> suffering on the second as well.
>>
>> Given the reactions here, it sounds like most of us are just as
>> unhappy this happened as the author, and would be happy to pitch in
>> and improve things.
>>
>> I agree with Kenn that just adding to more the contributor guide
>> always help because a contributor guide with everything one might need
>> to know is the least likely to actually be read in its entirety.
>> Rather it's useful to provide such guidance at the point that it's
>> needed. Specifically, I would like to see
>>
>> (1) A sentence in the PR template suggesting adding a reviewer. (easy)
>> (2) An automated recommendation for suggesting good candidate
>> reviewers (if we deem Github's suggestions insufficient). (harder)
>> (3) A bot that follows up on PRs after 1 week(?) noting the lack of
>> action and suggesting (and, implicitly but importantly
>> permission/expectation) that the author bring the PR to the list.
>> (medium)
>>
>> We could also have automated emails like the Beam Dependency Check
>> Report, but automated emails are much easier to ignore than personal
>> ones. Having the author ping dev@ has the added advantage that it
>> gives the author something they can do to move the PR forward, and
>> provides a clear signal that this is a PR someone cares enough about
>> to prioritize it above others. (It's certainly disheartening as a
>> reviewer to put time into reviewing a PR and then the author doesn't
>> bother to even respond, or (as has happened to me) be told "hey, this
>> wasn't ready for review yet." On the other hand it's rewarding to help
>> someone, especially a first time contributor, to see their change
>> actually get in. Improving this ratio will I think both increase the
>> productivity of reviews and the motivation to do them.)
>>
>> > Also, I started to review the PR on Friday (thx Kenn for pinging me).
>> >
>> > Etienne
>> >
>> > Le vendredi 25 janvier 2019 à 10:21 -0800, Rui Wang a écrit :
>> >
>> > 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.dev@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 <ie...@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 <
>> timrobertson100@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 <ke...@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
>> >
>> >
>>
>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Łukasz Gajowy <lg...@apache.org>.
IMHO, I don't think committers spend time watching new PRs coming up, but
they more likely act when pinged. So, we may need some automation in case a
contributor do not use github reviewed proposal. Auto reviewer assignment
seem too much but modifying the PR template to add a sentence such as
"please pickup a reviewer in the proposed list" could be enough.
WDYT ?

and

(1) A sentence in the PR template suggesting adding a reviewer. (easy)


+100! Let's improve the message in the PR template. It costs nothing and
can help a lot. I'd say it should be in bold letters as this is super
important.

Maybe this is also worth reconsidering if auto reviewer assignment (or at
least some form of it) is a bad idea. We can assign committers (the most
"hardcore" option, maybe too much) or ping them in emails/github comments
if there's inactivity in pull requests (the soft one but requires a bot to
be implemented). The way I see this is that such auto assigned reviewer
could always say "I have lots on my plate" but suggest someone else to take
care of the PR. This way the problem that nobody is mentioned by the PR
author is completely gone. Other than that, such an approach feels
efficient to me because it's more "in person" (similar to what Robert
said).

It's certainly disheartening as a
reviewer to put time into reviewing a PR and then the author doesn't
bother to even respond, or (as has happened to me) be told "hey, this
wasn't ready for review yet."

As for "this wasn't ready for review" - there are sometimes situations that
require a PR to be opened before they are actually completed (especially
when working with Jenkins jobs). Given that there might be
misunderstandings authors of such commits should give a clear message
saying "do not merge yet" or "not ready for review" in title or comments or
even close such PR and reopen until the change is ready. It's all about
giving a clear signal to others.

Maybe we should mention it in guidelines/PR message too to avoid situations
like this?

Łukasz


pon., 28 sty 2019 o 11:30 Robert Bradshaw <ro...@google.com> napisał(a):

> On Mon, Jan 28, 2019 at 10:37 AM Etienne Chauchot <ec...@apache.org>
> wrote:
> >
> > Sure it's a pity than this PR got unnoticed and I think it is a
> combination of factors (PR date around Christmas, the fact that the author
> forgot - AFAIK - to ping a reviewer in either the PR or the ML).
> >
> > I agree with Rui's proposal to enhance visibility of the "how to get a
> reviewed" process.
> >
> > IMHO, I don't think committers spend time watching new PRs coming up,
> but they more likely act when pinged. So, we may need some automation in
> case a contributor do not use github reviewed proposal. Auto reviewer
> assignment seem too much but modifying the PR template to add a sentence
> such as "please pickup a reviewer in the proposed list" could be enough.
> > WDYT ?
>
> +1
>
> I see two somewhat separable areas of improvement:
>
> (1) Getting a reviewer assigned to a PR, and
> (2) Expectations of feedback/timeliness from a reviewer once it has
> been assigned.
>
> The first is the motivation for this thread, but I think we're
> suffering on the second as well.
>
> Given the reactions here, it sounds like most of us are just as
> unhappy this happened as the author, and would be happy to pitch in
> and improve things.
>
> I agree with Kenn that just adding to more the contributor guide
> always help because a contributor guide with everything one might need
> to know is the least likely to actually be read in its entirety.
> Rather it's useful to provide such guidance at the point that it's
> needed. Specifically, I would like to see
>
> (1) A sentence in the PR template suggesting adding a reviewer. (easy)
> (2) An automated recommendation for suggesting good candidate
> reviewers (if we deem Github's suggestions insufficient). (harder)
> (3) A bot that follows up on PRs after 1 week(?) noting the lack of
> action and suggesting (and, implicitly but importantly
> permission/expectation) that the author bring the PR to the list.
> (medium)
>
> We could also have automated emails like the Beam Dependency Check
> Report, but automated emails are much easier to ignore than personal
> ones. Having the author ping dev@ has the added advantage that it
> gives the author something they can do to move the PR forward, and
> provides a clear signal that this is a PR someone cares enough about
> to prioritize it above others. (It's certainly disheartening as a
> reviewer to put time into reviewing a PR and then the author doesn't
> bother to even respond, or (as has happened to me) be told "hey, this
> wasn't ready for review yet." On the other hand it's rewarding to help
> someone, especially a first time contributor, to see their change
> actually get in. Improving this ratio will I think both increase the
> productivity of reviews and the motivation to do them.)
>
> > Also, I started to review the PR on Friday (thx Kenn for pinging me).
> >
> > Etienne
> >
> > Le vendredi 25 janvier 2019 à 10:21 -0800, Rui Wang a écrit :
> >
> > 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.dev@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 <ie...@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 <ti...@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 <ke...@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
> >
> >
>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Robert Bradshaw <ro...@google.com>.
On Mon, Jan 28, 2019 at 10:37 AM Etienne Chauchot <ec...@apache.org> wrote:
>
> Sure it's a pity than this PR got unnoticed and I think it is a combination of factors (PR date around Christmas, the fact that the author forgot - AFAIK - to ping a reviewer in either the PR or the ML).
>
> I agree with Rui's proposal to enhance visibility of the "how to get a reviewed" process.
>
> IMHO, I don't think committers spend time watching new PRs coming up, but they more likely act when pinged. So, we may need some automation in case a contributor do not use github reviewed proposal. Auto reviewer assignment seem too much but modifying the PR template to add a sentence such as "please pickup a reviewer in the proposed list" could be enough.
> WDYT ?

+1

I see two somewhat separable areas of improvement:

(1) Getting a reviewer assigned to a PR, and
(2) Expectations of feedback/timeliness from a reviewer once it has
been assigned.

The first is the motivation for this thread, but I think we're
suffering on the second as well.

Given the reactions here, it sounds like most of us are just as
unhappy this happened as the author, and would be happy to pitch in
and improve things.

I agree with Kenn that just adding to more the contributor guide
always help because a contributor guide with everything one might need
to know is the least likely to actually be read in its entirety.
Rather it's useful to provide such guidance at the point that it's
needed. Specifically, I would like to see

(1) A sentence in the PR template suggesting adding a reviewer. (easy)
(2) An automated recommendation for suggesting good candidate
reviewers (if we deem Github's suggestions insufficient). (harder)
(3) A bot that follows up on PRs after 1 week(?) noting the lack of
action and suggesting (and, implicitly but importantly
permission/expectation) that the author bring the PR to the list.
(medium)

We could also have automated emails like the Beam Dependency Check
Report, but automated emails are much easier to ignore than personal
ones. Having the author ping dev@ has the added advantage that it
gives the author something they can do to move the PR forward, and
provides a clear signal that this is a PR someone cares enough about
to prioritize it above others. (It's certainly disheartening as a
reviewer to put time into reviewing a PR and then the author doesn't
bother to even respond, or (as has happened to me) be told "hey, this
wasn't ready for review yet." On the other hand it's rewarding to help
someone, especially a first time contributor, to see their change
actually get in. Improving this ratio will I think both increase the
productivity of reviews and the motivation to do them.)

> Also, I started to review the PR on Friday (thx Kenn for pinging me).
>
> Etienne
>
> Le vendredi 25 janvier 2019 à 10:21 -0800, Rui Wang a écrit :
>
> 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 <ar...@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 <ie...@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 <ti...@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 <ke...@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
>
>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Etienne Chauchot <ec...@apache.org>.
Sure it's a pity than this PR got unnoticed and I think it is a combination of factors (PR date around Christmas, the
fact that the author forgot - AFAIK - to ping a reviewer in either the PR or the ML).
I agree with Rui's proposal to enhance visibility of the "how to get a reviewed" process.
IMHO, I don't think committers spend time watching new PRs coming up, but they more likely act when pinged. So, we may
need some automation in case a contributor do not use github reviewed proposal. Auto reviewer assignment seem too much
but modifying the PR template to add a sentence such as "please pickup a reviewer in the proposed list" could be
enough. WDYT ? 

Also, I started to review the PR on Friday (thx Kenn for pinging me).
Etienne
Le vendredi 25 janvier 2019 à 10:21 -0800, Rui Wang a écrit :
> 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 <ar...@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 <ie...@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 <ti...@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 <ke...@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

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Rui Wang <ru...@google.com>.
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 <ar...@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 <ie...@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 <ti...@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 <ke...@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
>
>
>

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Alexey Romanenko <ar...@gmail.com>.
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 <https://beam.apache.org/contribute/#make-your-change>


> On 25 Jan 2019, at 17:50, Ismaël Mejía <ie...@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 <ti...@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 <ke...@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


Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Ismaël Mejía <ie...@gmail.com>.
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 <ti...@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 <ke...@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

Re: BEAM-6324 / #7340: "I've pretty much given up on the PR being merged. I use my own fork for my projects"

Posted by Tim Robertson <ti...@gmail.com>.
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 <ke...@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
>