You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Fabian Hueske <fh...@gmail.com> on 2018/10/16 16:47:44 UTC
Re: [DISCUSS] [Contributing] (3) - Review Tooling
Hi everyone,
Instead of manually adding the review progress template as a comment to
every new PR, we could also append it to the PR description template.
The benefits would be:
+ no need to add it manually since it is automatically added to each PR
+ the template is versioned in the Flink Git repository
+ contributors can learn about the review process before opening a PR
On the downside, the template grows a bit at the end.
What do you think?
Best, Fabian
Am Mo., 24. Sep. 2018 um 15:51 Uhr schrieb Fabian Hueske <fhueske@gmail.com
>:
> Hi,
>
> Coming back to the original topic of the thread: How to implement the
> guided review process.
>
> I am in favor of starting with a low-tech solution.
> We design a review template with a checkbox for the five questions (see
> [1]) and a link to the detailed description of the review process ([1] will
> be added to flink.apache.org).
> Once a PR is opened, anybody (the PR author, a committer, any reviewer,
> ...) can post the review template as a comment. Ideally this happens
> shortly after the PR was opened.
> If we find it necessary, we can later add a bot to automate posting the
> template as comment.
>
> Once the template is posted, the PR can be reviewed by following the
> process and answering the template questions.
> When all boxes are ticked off, the PR can be merged.
>
> Best,
> Fabian
>
>
>
>
>
> [1]
> https://docs.google.com/document/d/1yaX2b9LNh-6LxrAmE23U3D2cRbocGlGKCYnvJd9lVhk/
>
>
> Am Mo., 24. Sep. 2018 um 12:27 Uhr schrieb vino yang <
> yanghua1127@gmail.com>:
>
>> Hi,
>>
>> About "valuable", I agree with @Aljoscha that there is no clear standard
>> of
>> judgment about "valuable".
>> But I think the priority may be a more specific indicator, because the
>> JIRA
>> issue also has a "Priority" attribute.
>> Maybe we can tag the PR, for example: use the "Label" function of github,
>> or add the "[Priority]" tag to the PR title?
>>
>> Regarding the closure of inactive PR, I feel that it is more cautious to
>> shut down artificially.
>> Whether it is possible to explicitly assign a PR to a committer familiar
>> with the module, which will reduce the unnecessary ping operation of many
>> contributors.
>> Because some people don't know which committer is familiar with the module
>> he changed.
>>
>> Thanks, vino.
>>
>> Aljoscha Krettek <al...@apache.org> 于2018年9月24日周一 下午5:03写道：
>>
>> > In Beam, we have a bot that regularly nags people about inactive PRs and
>> > also closes them after long inactivity.
>> >
>> > And we use the github feature for assigning reviewers in github.
>> >
>> > Sometimes it is hard for people to judge how "valuable" a PR is. Maybe
>> > some knowledgable people could mark PRs as valuable if they think it's a
>> > good addition but if they don't have the review bandwith. Other people
>> can
>> > then search for valuable PRs that don't yet a reviewer and review/merge
>> > them.
>> >
>> > Aljoscha
>> >
>> > > On 22. Sep 2018, at 04:25, vino yang <ya...@gmail.com> wrote:
>> > >
>> > > Hi Jin Sun,
>> > >
>> > > Earlier this year, I also had these questions when I started
>> contributing
>> > > code to Flink. In fact, the timing of a PR being reviewed will be
>> related
>> > > to the priority of the problem solved by the PR.
>> > > And when you indicate the module to which it belongs in the title of
>> the
>> > > PR, like "[FLINK-xxxx][module] XXXX", the person in charge of the
>> > relevant
>> > > module or the contributor who is familiar with it will find it easier.
>> > >
>> > > To Stephan:
>> > >
>> > > Maybe we can open a separate mail thread (after all, the current
>> > discussion
>> > > thread is about a specific topic) to hear the contributors about PR
>> > review
>> > > related questions and doubts. Perhaps some of their feedback will help
>> > the
>> > > community improve the way they review.
>> > >
>> > > Thanks, vino.
>> > >
>> > > Jin Sun <is...@gmail.com> 于2018年9月22日周六 上午6:40写道：
>> > >
>> > >> As a new contributor I cared about how to make my contribution
>> accepted
>> > by
>> > >> the community, some questions:
>> > >> 1) When will it get reviewed? Is there a rule about review timeline?
>> > >> 2) There are long backlog of pull requests, What happened if a pull
>> > >> request not get noticed, do we have some mechanism to make it moving
>> > >> forward, like a pull request will be assigned a owner of reviewer?
>> Or we
>> > >> have a review queue and a pull request will be get handled fairly.
>> > >>
>> > >> Jin
>> > >>
>> > >>
>> > >>> On Sep 20, 2018, at 12:56 PM, Stephan Ewen <se...@apache.org>
>> wrote:
>> > >>>
>> > >>> Hi all!
>> > >>>
>> > >>> This thread is dedicated to discuss the tooling we want to use for
>> the
>> > >>> reviews.
>> > >>> It is spun out of the proposal *"A more structured approach to
>> reviews
>> > >> and
>> > >>> contributions".*
>> > >>>
>> > >>>
>> > >>> *Suggestions brought up so far*
>> > >>>
>> > >>>
>> > >>> *Use comments / template with checklist*
>> > >>>
>> > >>> - Easy to do
>> > >>> - Manual, a bit of reviewer overhead, reviewers needs to know the
>> > >> process
>> > >>>
>> > >>> *Use a bot *
>> > >>>
>> > >>> - Automatically add the review questions to each new PR
>> > >>> - Further details?
>> > >>>
>> > >>> *Use GitHub labels*
>> > >>>
>> > >>> - Searchable
>> > >>> - possibly not obvious to new contributors
>> > >>> - Any restrictions? Do members need to apply at ASF infra to have
>> > >>> permissions to edit github labels?
>> > >>
>> > >>
>> >
>> >
>>
>
Re: [DISCUSS] [Contributing] (3) - Review Tooling
Posted by Hequn Cheng <ch...@gmail.com>.
Hi,
I'm slightly prefer the bot option. The bot can post the review template
automatically. But I do agree that we can start with a low-tech solution
and add a bot later if find it helpful.
Best, Hequn
On Wed, Oct 17, 2018 at 11:17 AM Jin Sun <is...@gmail.com> wrote:
> +1
>
> On Tue, Oct 16, 2018 at 7:51 PM Tzu-Li Chen <wa...@gmail.com> wrote:
>
> > Hi Fabian,
> >
> > +1 for your proposal.
> >
> > For the downside, I think after adding the review process template,
> > the pull request template would be high level into 3 parts:
> >
> > 1. Greeting and community guiding.
> > 2. User completed template.
> > 3. Reviewer complete template.
> >
> > If we can visually separate them, i.e., help a new contributor regard the
> > whole template into 3 parts, I think this downside is not so critical.
> For
> > some previous attempt, see also [1].
> >
> > Best,
> > tison.
> >
> > [1] https://github.com/apache/flink/pull/6722
> >
> >
> > vino yang <ya...@gmail.com> 于2018年10月17日周三 上午9:57写道：
> >
> > > +1,
> > >
> > > Agree to use the PR template.
> > >
> > > Fabian Hueske <fh...@gmail.com> 于2018年10月17日周三 上午12:48写道：
> > >
> > > > Hi everyone,
> > > >
> > > > Instead of manually adding the review progress template as a comment
> to
> > > > every new PR, we could also append it to the PR description template.
> > > > The benefits would be:
> > > > + no need to add it manually since it is automatically added to each
> PR
> > > > + the template is versioned in the Flink Git repository
> > > > + contributors can learn about the review process before opening a PR
> > > >
> > > > On the downside, the template grows a bit at the end.
> > > >
> > > > What do you think?
> > > >
> > > > Best, Fabian
> > > >
> > > > Am Mo., 24. Sep. 2018 um 15:51 Uhr schrieb Fabian Hueske <
> > > > fhueske@gmail.com
> > > > >:
> > > >
> > > > > Hi,
> > > > >
> > > > > Coming back to the original topic of the thread: How to implement
> the
> > > > > guided review process.
> > > > >
> > > > > I am in favor of starting with a low-tech solution.
> > > > > We design a review template with a checkbox for the five questions
> > (see
> > > > > [1]) and a link to the detailed description of the review process
> > ([1]
> > > > will
> > > > > be added to flink.apache.org).
> > > > > Once a PR is opened, anybody (the PR author, a committer, any
> > reviewer,
> > > > > ...) can post the review template as a comment. Ideally this
> happens
> > > > > shortly after the PR was opened.
> > > > > If we find it necessary, we can later add a bot to automate posting
> > the
> > > > > template as comment.
> > > > >
> > > > > Once the template is posted, the PR can be reviewed by following
> the
> > > > > process and answering the template questions.
> > > > > When all boxes are ticked off, the PR can be merged.
> > > > >
> > > > > Best,
> > > > > Fabian
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > [1]
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/1yaX2b9LNh-6LxrAmE23U3D2cRbocGlGKCYnvJd9lVhk/
> > > > >
> > > > >
> > > > > Am Mo., 24. Sep. 2018 um 12:27 Uhr schrieb vino yang <
> > > > > yanghua1127@gmail.com>:
> > > > >
> > > > >> Hi,
> > > > >>
> > > > >> About "valuable", I agree with @Aljoscha that there is no clear
> > > standard
> > > > >> of
> > > > >> judgment about "valuable".
> > > > >> But I think the priority may be a more specific indicator, because
> > the
> > > > >> JIRA
> > > > >> issue also has a "Priority" attribute.
> > > > >> Maybe we can tag the PR, for example: use the "Label" function of
> > > > github,
> > > > >> or add the "[Priority]" tag to the PR title?
> > > > >>
> > > > >> Regarding the closure of inactive PR, I feel that it is more
> > cautious
> > > to
> > > > >> shut down artificially.
> > > > >> Whether it is possible to explicitly assign a PR to a committer
> > > familiar
> > > > >> with the module, which will reduce the unnecessary ping operation
> of
> > > > many
> > > > >> contributors.
> > > > >> Because some people don't know which committer is familiar with
> the
> > > > module
> > > > >> he changed.
> > > > >>
> > > > >> Thanks, vino.
> > > > >>
> > > > >> Aljoscha Krettek <al...@apache.org> 于2018年9月24日周一 下午5:03写道：
> > > > >>
> > > > >> > In Beam, we have a bot that regularly nags people about inactive
> > PRs
> > > > and
> > > > >> > also closes them after long inactivity.
> > > > >> >
> > > > >> > And we use the github feature for assigning reviewers in github.
> > > > >> >
> > > > >> > Sometimes it is hard for people to judge how "valuable" a PR is.
> > > Maybe
> > > > >> > some knowledgable people could mark PRs as valuable if they
> think
> > > > it's a
> > > > >> > good addition but if they don't have the review bandwith. Other
> > > people
> > > > >> can
> > > > >> > then search for valuable PRs that don't yet a reviewer and
> > > > review/merge
> > > > >> > them.
> > > > >> >
> > > > >> > Aljoscha
> > > > >> >
> > > > >> > > On 22. Sep 2018, at 04:25, vino yang <ya...@gmail.com>
> > > wrote:
> > > > >> > >
> > > > >> > > Hi Jin Sun,
> > > > >> > >
> > > > >> > > Earlier this year, I also had these questions when I started
> > > > >> contributing
> > > > >> > > code to Flink. In fact, the timing of a PR being reviewed will
> > be
> > > > >> related
> > > > >> > > to the priority of the problem solved by the PR.
> > > > >> > > And when you indicate the module to which it belongs in the
> > title
> > > of
> > > > >> the
> > > > >> > > PR, like "[FLINK-xxxx][module] XXXX", the person in charge of
> > the
> > > > >> > relevant
> > > > >> > > module or the contributor who is familiar with it will find it
> > > > easier.
> > > > >> > >
> > > > >> > > To Stephan:
> > > > >> > >
> > > > >> > > Maybe we can open a separate mail thread (after all, the
> current
> > > > >> > discussion
> > > > >> > > thread is about a specific topic) to hear the contributors
> about
> > > PR
> > > > >> > review
> > > > >> > > related questions and doubts. Perhaps some of their feedback
> > will
> > > > help
> > > > >> > the
> > > > >> > > community improve the way they review.
> > > > >> > >
> > > > >> > > Thanks, vino.
> > > > >> > >
> > > > >> > > Jin Sun <is...@gmail.com> 于2018年9月22日周六 上午6:40写道：
> > > > >> > >
> > > > >> > >> As a new contributor I cared about how to make my
> contribution
> > > > >> accepted
> > > > >> > by
> > > > >> > >> the community, some questions:
> > > > >> > >> 1) When will it get reviewed? Is there a rule about review
> > > > timeline?
> > > > >> > >> 2) There are long backlog of pull requests, What happened if
> a
> > > pull
> > > > >> > >> request not get noticed, do we have some mechanism to make it
> > > > moving
> > > > >> > >> forward, like a pull request will be assigned a owner of
> > > reviewer?
> > > > >> Or we
> > > > >> > >> have a review queue and a pull request will be get handled
> > > fairly.
> > > > >> > >>
> > > > >> > >> Jin
> > > > >> > >>
> > > > >> > >>
> > > > >> > >>> On Sep 20, 2018, at 12:56 PM, Stephan Ewen <
> sewen@apache.org>
> > > > >> wrote:
> > > > >> > >>>
> > > > >> > >>> Hi all!
> > > > >> > >>>
> > > > >> > >>> This thread is dedicated to discuss the tooling we want to
> use
> > > for
> > > > >> the
> > > > >> > >>> reviews.
> > > > >> > >>> It is spun out of the proposal *"A more structured approach
> to
> > > > >> reviews
> > > > >> > >> and
> > > > >> > >>> contributions".*
> > > > >> > >>>
> > > > >> > >>>
> > > > >> > >>> *Suggestions brought up so far*
> > > > >> > >>>
> > > > >> > >>>
> > > > >> > >>> *Use comments / template with checklist*
> > > > >> > >>>
> > > > >> > >>> - Easy to do
> > > > >> > >>> - Manual, a bit of reviewer overhead, reviewers needs to
> know
> > > the
> > > > >> > >> process
> > > > >> > >>>
> > > > >> > >>> *Use a bot *
> > > > >> > >>>
> > > > >> > >>> - Automatically add the review questions to each new PR
> > > > >> > >>> - Further details?
> > > > >> > >>>
> > > > >> > >>> *Use GitHub labels*
> > > > >> > >>>
> > > > >> > >>> - Searchable
> > > > >> > >>> - possibly not obvious to new contributors
> > > > >> > >>> - Any restrictions? Do members need to apply at ASF infra to
> > > have
> > > > >> > >>> permissions to edit github labels?
> > > > >> > >>
> > > > >> > >>
> > > > >> >
> > > > >> >
> > > > >>
> > > > >
> > > >
> > >
> >
>
>
> --
> Thanks,
> Jin SUN
>
Re: [DISCUSS] [Contributing] (3) - Review Tooling
Posted by Jin Sun <is...@gmail.com>.
+1
On Tue, Oct 16, 2018 at 7:51 PM Tzu-Li Chen <wa...@gmail.com> wrote:
> Hi Fabian,
>
> +1 for your proposal.
>
> For the downside, I think after adding the review process template,
> the pull request template would be high level into 3 parts:
>
> 1. Greeting and community guiding.
> 2. User completed template.
> 3. Reviewer complete template.
>
> If we can visually separate them, i.e., help a new contributor regard the
> whole template into 3 parts, I think this downside is not so critical. For
> some previous attempt, see also [1].
>
> Best,
> tison.
>
> [1] https://github.com/apache/flink/pull/6722
>
>
> vino yang <ya...@gmail.com> 于2018年10月17日周三 上午9:57写道：
>
> > +1,
> >
> > Agree to use the PR template.
> >
> > Fabian Hueske <fh...@gmail.com> 于2018年10月17日周三 上午12:48写道：
> >
> > > Hi everyone,
> > >
> > > Instead of manually adding the review progress template as a comment to
> > > every new PR, we could also append it to the PR description template.
> > > The benefits would be:
> > > + no need to add it manually since it is automatically added to each PR
> > > + the template is versioned in the Flink Git repository
> > > + contributors can learn about the review process before opening a PR
> > >
> > > On the downside, the template grows a bit at the end.
> > >
> > > What do you think?
> > >
> > > Best, Fabian
> > >
> > > Am Mo., 24. Sep. 2018 um 15:51 Uhr schrieb Fabian Hueske <
> > > fhueske@gmail.com
> > > >:
> > >
> > > > Hi,
> > > >
> > > > Coming back to the original topic of the thread: How to implement the
> > > > guided review process.
> > > >
> > > > I am in favor of starting with a low-tech solution.
> > > > We design a review template with a checkbox for the five questions
> (see
> > > > [1]) and a link to the detailed description of the review process
> ([1]
> > > will
> > > > be added to flink.apache.org).
> > > > Once a PR is opened, anybody (the PR author, a committer, any
> reviewer,
> > > > ...) can post the review template as a comment. Ideally this happens
> > > > shortly after the PR was opened.
> > > > If we find it necessary, we can later add a bot to automate posting
> the
> > > > template as comment.
> > > >
> > > > Once the template is posted, the PR can be reviewed by following the
> > > > process and answering the template questions.
> > > > When all boxes are ticked off, the PR can be merged.
> > > >
> > > > Best,
> > > > Fabian
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > [1]
> > > >
> > >
> >
> https://docs.google.com/document/d/1yaX2b9LNh-6LxrAmE23U3D2cRbocGlGKCYnvJd9lVhk/
> > > >
> > > >
> > > > Am Mo., 24. Sep. 2018 um 12:27 Uhr schrieb vino yang <
> > > > yanghua1127@gmail.com>:
> > > >
> > > >> Hi,
> > > >>
> > > >> About "valuable", I agree with @Aljoscha that there is no clear
> > standard
> > > >> of
> > > >> judgment about "valuable".
> > > >> But I think the priority may be a more specific indicator, because
> the
> > > >> JIRA
> > > >> issue also has a "Priority" attribute.
> > > >> Maybe we can tag the PR, for example: use the "Label" function of
> > > github,
> > > >> or add the "[Priority]" tag to the PR title?
> > > >>
> > > >> Regarding the closure of inactive PR, I feel that it is more
> cautious
> > to
> > > >> shut down artificially.
> > > >> Whether it is possible to explicitly assign a PR to a committer
> > familiar
> > > >> with the module, which will reduce the unnecessary ping operation of
> > > many
> > > >> contributors.
> > > >> Because some people don't know which committer is familiar with the
> > > module
> > > >> he changed.
> > > >>
> > > >> Thanks, vino.
> > > >>
> > > >> Aljoscha Krettek <al...@apache.org> 于2018年9月24日周一 下午5:03写道：
> > > >>
> > > >> > In Beam, we have a bot that regularly nags people about inactive
> PRs
> > > and
> > > >> > also closes them after long inactivity.
> > > >> >
> > > >> > And we use the github feature for assigning reviewers in github.
> > > >> >
> > > >> > Sometimes it is hard for people to judge how "valuable" a PR is.
> > Maybe
> > > >> > some knowledgable people could mark PRs as valuable if they think
> > > it's a
> > > >> > good addition but if they don't have the review bandwith. Other
> > people
> > > >> can
> > > >> > then search for valuable PRs that don't yet a reviewer and
> > > review/merge
> > > >> > them.
> > > >> >
> > > >> > Aljoscha
> > > >> >
> > > >> > > On 22. Sep 2018, at 04:25, vino yang <ya...@gmail.com>
> > wrote:
> > > >> > >
> > > >> > > Hi Jin Sun,
> > > >> > >
> > > >> > > Earlier this year, I also had these questions when I started
> > > >> contributing
> > > >> > > code to Flink. In fact, the timing of a PR being reviewed will
> be
> > > >> related
> > > >> > > to the priority of the problem solved by the PR.
> > > >> > > And when you indicate the module to which it belongs in the
> title
> > of
> > > >> the
> > > >> > > PR, like "[FLINK-xxxx][module] XXXX", the person in charge of
> the
> > > >> > relevant
> > > >> > > module or the contributor who is familiar with it will find it
> > > easier.
> > > >> > >
> > > >> > > To Stephan:
> > > >> > >
> > > >> > > Maybe we can open a separate mail thread (after all, the current
> > > >> > discussion
> > > >> > > thread is about a specific topic) to hear the contributors about
> > PR
> > > >> > review
> > > >> > > related questions and doubts. Perhaps some of their feedback
> will
> > > help
> > > >> > the
> > > >> > > community improve the way they review.
> > > >> > >
> > > >> > > Thanks, vino.
> > > >> > >
> > > >> > > Jin Sun <is...@gmail.com> 于2018年9月22日周六 上午6:40写道：
> > > >> > >
> > > >> > >> As a new contributor I cared about how to make my contribution
> > > >> accepted
> > > >> > by
> > > >> > >> the community, some questions:
> > > >> > >> 1) When will it get reviewed? Is there a rule about review
> > > timeline?
> > > >> > >> 2) There are long backlog of pull requests, What happened if a
> > pull
> > > >> > >> request not get noticed, do we have some mechanism to make it
> > > moving
> > > >> > >> forward, like a pull request will be assigned a owner of
> > reviewer?
> > > >> Or we
> > > >> > >> have a review queue and a pull request will be get handled
> > fairly.
> > > >> > >>
> > > >> > >> Jin
> > > >> > >>
> > > >> > >>
> > > >> > >>> On Sep 20, 2018, at 12:56 PM, Stephan Ewen <se...@apache.org>
> > > >> wrote:
> > > >> > >>>
> > > >> > >>> Hi all!
> > > >> > >>>
> > > >> > >>> This thread is dedicated to discuss the tooling we want to use
> > for
> > > >> the
> > > >> > >>> reviews.
> > > >> > >>> It is spun out of the proposal *"A more structured approach to
> > > >> reviews
> > > >> > >> and
> > > >> > >>> contributions".*
> > > >> > >>>
> > > >> > >>>
> > > >> > >>> *Suggestions brought up so far*
> > > >> > >>>
> > > >> > >>>
> > > >> > >>> *Use comments / template with checklist*
> > > >> > >>>
> > > >> > >>> - Easy to do
> > > >> > >>> - Manual, a bit of reviewer overhead, reviewers needs to know
> > the
> > > >> > >> process
> > > >> > >>>
> > > >> > >>> *Use a bot *
> > > >> > >>>
> > > >> > >>> - Automatically add the review questions to each new PR
> > > >> > >>> - Further details?
> > > >> > >>>
> > > >> > >>> *Use GitHub labels*
> > > >> > >>>
> > > >> > >>> - Searchable
> > > >> > >>> - possibly not obvious to new contributors
> > > >> > >>> - Any restrictions? Do members need to apply at ASF infra to
> > have
> > > >> > >>> permissions to edit github labels?
> > > >> > >>
> > > >> > >>
> > > >> >
> > > >> >
> > > >>
> > > >
> > >
> >
>
--
Thanks,
Jin SUN
Re: [DISCUSS] [Contributing] (3) - Review Tooling
Posted by Tzu-Li Chen <wa...@gmail.com>.
Hi Fabian,
+1 for your proposal.
For the downside, I think after adding the review process template,
the pull request template would be high level into 3 parts:
1. Greeting and community guiding.
2. User completed template.
3. Reviewer complete template.
If we can visually separate them, i.e., help a new contributor regard the
whole template into 3 parts, I think this downside is not so critical. For
some previous attempt, see also [1].
Best,
tison.
[1] https://github.com/apache/flink/pull/6722
vino yang <ya...@gmail.com> 于2018年10月17日周三 上午9:57写道：
> +1,
>
> Agree to use the PR template.
>
> Fabian Hueske <fh...@gmail.com> 于2018年10月17日周三 上午12:48写道：
>
> > Hi everyone,
> >
> > Instead of manually adding the review progress template as a comment to
> > every new PR, we could also append it to the PR description template.
> > The benefits would be:
> > + no need to add it manually since it is automatically added to each PR
> > + the template is versioned in the Flink Git repository
> > + contributors can learn about the review process before opening a PR
> >
> > On the downside, the template grows a bit at the end.
> >
> > What do you think?
> >
> > Best, Fabian
> >
> > Am Mo., 24. Sep. 2018 um 15:51 Uhr schrieb Fabian Hueske <
> > fhueske@gmail.com
> > >:
> >
> > > Hi,
> > >
> > > Coming back to the original topic of the thread: How to implement the
> > > guided review process.
> > >
> > > I am in favor of starting with a low-tech solution.
> > > We design a review template with a checkbox for the five questions (see
> > > [1]) and a link to the detailed description of the review process ([1]
> > will
> > > be added to flink.apache.org).
> > > Once a PR is opened, anybody (the PR author, a committer, any reviewer,
> > > ...) can post the review template as a comment. Ideally this happens
> > > shortly after the PR was opened.
> > > If we find it necessary, we can later add a bot to automate posting the
> > > template as comment.
> > >
> > > Once the template is posted, the PR can be reviewed by following the
> > > process and answering the template questions.
> > > When all boxes are ticked off, the PR can be merged.
> > >
> > > Best,
> > > Fabian
> > >
> > >
> > >
> > >
> > >
> > > [1]
> > >
> >
> https://docs.google.com/document/d/1yaX2b9LNh-6LxrAmE23U3D2cRbocGlGKCYnvJd9lVhk/
> > >
> > >
> > > Am Mo., 24. Sep. 2018 um 12:27 Uhr schrieb vino yang <
> > > yanghua1127@gmail.com>:
> > >
> > >> Hi,
> > >>
> > >> About "valuable", I agree with @Aljoscha that there is no clear
> standard
> > >> of
> > >> judgment about "valuable".
> > >> But I think the priority may be a more specific indicator, because the
> > >> JIRA
> > >> issue also has a "Priority" attribute.
> > >> Maybe we can tag the PR, for example: use the "Label" function of
> > github,
> > >> or add the "[Priority]" tag to the PR title?
> > >>
> > >> Regarding the closure of inactive PR, I feel that it is more cautious
> to
> > >> shut down artificially.
> > >> Whether it is possible to explicitly assign a PR to a committer
> familiar
> > >> with the module, which will reduce the unnecessary ping operation of
> > many
> > >> contributors.
> > >> Because some people don't know which committer is familiar with the
> > module
> > >> he changed.
> > >>
> > >> Thanks, vino.
> > >>
> > >> Aljoscha Krettek <al...@apache.org> 于2018年9月24日周一 下午5:03写道：
> > >>
> > >> > In Beam, we have a bot that regularly nags people about inactive PRs
> > and
> > >> > also closes them after long inactivity.
> > >> >
> > >> > And we use the github feature for assigning reviewers in github.
> > >> >
> > >> > Sometimes it is hard for people to judge how "valuable" a PR is.
> Maybe
> > >> > some knowledgable people could mark PRs as valuable if they think
> > it's a
> > >> > good addition but if they don't have the review bandwith. Other
> people
> > >> can
> > >> > then search for valuable PRs that don't yet a reviewer and
> > review/merge
> > >> > them.
> > >> >
> > >> > Aljoscha
> > >> >
> > >> > > On 22. Sep 2018, at 04:25, vino yang <ya...@gmail.com>
> wrote:
> > >> > >
> > >> > > Hi Jin Sun,
> > >> > >
> > >> > > Earlier this year, I also had these questions when I started
> > >> contributing
> > >> > > code to Flink. In fact, the timing of a PR being reviewed will be
> > >> related
> > >> > > to the priority of the problem solved by the PR.
> > >> > > And when you indicate the module to which it belongs in the title
> of
> > >> the
> > >> > > PR, like "[FLINK-xxxx][module] XXXX", the person in charge of the
> > >> > relevant
> > >> > > module or the contributor who is familiar with it will find it
> > easier.
> > >> > >
> > >> > > To Stephan:
> > >> > >
> > >> > > Maybe we can open a separate mail thread (after all, the current
> > >> > discussion
> > >> > > thread is about a specific topic) to hear the contributors about
> PR
> > >> > review
> > >> > > related questions and doubts. Perhaps some of their feedback will
> > help
> > >> > the
> > >> > > community improve the way they review.
> > >> > >
> > >> > > Thanks, vino.
> > >> > >
> > >> > > Jin Sun <is...@gmail.com> 于2018年9月22日周六 上午6:40写道：
> > >> > >
> > >> > >> As a new contributor I cared about how to make my contribution
> > >> accepted
> > >> > by
> > >> > >> the community, some questions:
> > >> > >> 1) When will it get reviewed? Is there a rule about review
> > timeline?
> > >> > >> 2) There are long backlog of pull requests, What happened if a
> pull
> > >> > >> request not get noticed, do we have some mechanism to make it
> > moving
> > >> > >> forward, like a pull request will be assigned a owner of
> reviewer?
> > >> Or we
> > >> > >> have a review queue and a pull request will be get handled
> fairly.
> > >> > >>
> > >> > >> Jin
> > >> > >>
> > >> > >>
> > >> > >>> On Sep 20, 2018, at 12:56 PM, Stephan Ewen <se...@apache.org>
> > >> wrote:
> > >> > >>>
> > >> > >>> Hi all!
> > >> > >>>
> > >> > >>> This thread is dedicated to discuss the tooling we want to use
> for
> > >> the
> > >> > >>> reviews.
> > >> > >>> It is spun out of the proposal *"A more structured approach to
> > >> reviews
> > >> > >> and
> > >> > >>> contributions".*
> > >> > >>>
> > >> > >>>
> > >> > >>> *Suggestions brought up so far*
> > >> > >>>
> > >> > >>>
> > >> > >>> *Use comments / template with checklist*
> > >> > >>>
> > >> > >>> - Easy to do
> > >> > >>> - Manual, a bit of reviewer overhead, reviewers needs to know
> the
> > >> > >> process
> > >> > >>>
> > >> > >>> *Use a bot *
> > >> > >>>
> > >> > >>> - Automatically add the review questions to each new PR
> > >> > >>> - Further details?
> > >> > >>>
> > >> > >>> *Use GitHub labels*
> > >> > >>>
> > >> > >>> - Searchable
> > >> > >>> - possibly not obvious to new contributors
> > >> > >>> - Any restrictions? Do members need to apply at ASF infra to
> have
> > >> > >>> permissions to edit github labels?
> > >> > >>
> > >> > >>
> > >> >
> > >> >
> > >>
> > >
> >
>
Re: [DISCUSS] [Contributing] (3) - Review Tooling
Posted by vino yang <ya...@gmail.com>.
+1,
Agree to use the PR template.
Fabian Hueske <fh...@gmail.com> 于2018年10月17日周三 上午12:48写道：
> Hi everyone,
>
> Instead of manually adding the review progress template as a comment to
> every new PR, we could also append it to the PR description template.
> The benefits would be:
> + no need to add it manually since it is automatically added to each PR
> + the template is versioned in the Flink Git repository
> + contributors can learn about the review process before opening a PR
>
> On the downside, the template grows a bit at the end.
>
> What do you think?
>
> Best, Fabian
>
> Am Mo., 24. Sep. 2018 um 15:51 Uhr schrieb Fabian Hueske <
> fhueske@gmail.com
> >:
>
> > Hi,
> >
> > Coming back to the original topic of the thread: How to implement the
> > guided review process.
> >
> > I am in favor of starting with a low-tech solution.
> > We design a review template with a checkbox for the five questions (see
> > [1]) and a link to the detailed description of the review process ([1]
> will
> > be added to flink.apache.org).
> > Once a PR is opened, anybody (the PR author, a committer, any reviewer,
> > ...) can post the review template as a comment. Ideally this happens
> > shortly after the PR was opened.
> > If we find it necessary, we can later add a bot to automate posting the
> > template as comment.
> >
> > Once the template is posted, the PR can be reviewed by following the
> > process and answering the template questions.
> > When all boxes are ticked off, the PR can be merged.
> >
> > Best,
> > Fabian
> >
> >
> >
> >
> >
> > [1]
> >
> https://docs.google.com/document/d/1yaX2b9LNh-6LxrAmE23U3D2cRbocGlGKCYnvJd9lVhk/
> >
> >
> > Am Mo., 24. Sep. 2018 um 12:27 Uhr schrieb vino yang <
> > yanghua1127@gmail.com>:
> >
> >> Hi,
> >>
> >> About "valuable", I agree with @Aljoscha that there is no clear standard
> >> of
> >> judgment about "valuable".
> >> But I think the priority may be a more specific indicator, because the
> >> JIRA
> >> issue also has a "Priority" attribute.
> >> Maybe we can tag the PR, for example: use the "Label" function of
> github,
> >> or add the "[Priority]" tag to the PR title?
> >>
> >> Regarding the closure of inactive PR, I feel that it is more cautious to
> >> shut down artificially.
> >> Whether it is possible to explicitly assign a PR to a committer familiar
> >> with the module, which will reduce the unnecessary ping operation of
> many
> >> contributors.
> >> Because some people don't know which committer is familiar with the
> module
> >> he changed.
> >>
> >> Thanks, vino.
> >>
> >> Aljoscha Krettek <al...@apache.org> 于2018年9月24日周一 下午5:03写道：
> >>
> >> > In Beam, we have a bot that regularly nags people about inactive PRs
> and
> >> > also closes them after long inactivity.
> >> >
> >> > And we use the github feature for assigning reviewers in github.
> >> >
> >> > Sometimes it is hard for people to judge how "valuable" a PR is. Maybe
> >> > some knowledgable people could mark PRs as valuable if they think
> it's a
> >> > good addition but if they don't have the review bandwith. Other people
> >> can
> >> > then search for valuable PRs that don't yet a reviewer and
> review/merge
> >> > them.
> >> >
> >> > Aljoscha
> >> >
> >> > > On 22. Sep 2018, at 04:25, vino yang <ya...@gmail.com> wrote:
> >> > >
> >> > > Hi Jin Sun,
> >> > >
> >> > > Earlier this year, I also had these questions when I started
> >> contributing
> >> > > code to Flink. In fact, the timing of a PR being reviewed will be
> >> related
> >> > > to the priority of the problem solved by the PR.
> >> > > And when you indicate the module to which it belongs in the title of
> >> the
> >> > > PR, like "[FLINK-xxxx][module] XXXX", the person in charge of the
> >> > relevant
> >> > > module or the contributor who is familiar with it will find it
> easier.
> >> > >
> >> > > To Stephan:
> >> > >
> >> > > Maybe we can open a separate mail thread (after all, the current
> >> > discussion
> >> > > thread is about a specific topic) to hear the contributors about PR
> >> > review
> >> > > related questions and doubts. Perhaps some of their feedback will
> help
> >> > the
> >> > > community improve the way they review.
> >> > >
> >> > > Thanks, vino.
> >> > >
> >> > > Jin Sun <is...@gmail.com> 于2018年9月22日周六 上午6:40写道：
> >> > >
> >> > >> As a new contributor I cared about how to make my contribution
> >> accepted
> >> > by
> >> > >> the community, some questions:
> >> > >> 1) When will it get reviewed? Is there a rule about review
> timeline?
> >> > >> 2) There are long backlog of pull requests, What happened if a pull
> >> > >> request not get noticed, do we have some mechanism to make it
> moving
> >> > >> forward, like a pull request will be assigned a owner of reviewer?
> >> Or we
> >> > >> have a review queue and a pull request will be get handled fairly.
> >> > >>
> >> > >> Jin
> >> > >>
> >> > >>
> >> > >>> On Sep 20, 2018, at 12:56 PM, Stephan Ewen <se...@apache.org>
> >> wrote:
> >> > >>>
> >> > >>> Hi all!
> >> > >>>
> >> > >>> This thread is dedicated to discuss the tooling we want to use for
> >> the
> >> > >>> reviews.
> >> > >>> It is spun out of the proposal *"A more structured approach to
> >> reviews
> >> > >> and
> >> > >>> contributions".*
> >> > >>>
> >> > >>>
> >> > >>> *Suggestions brought up so far*
> >> > >>>
> >> > >>>
> >> > >>> *Use comments / template with checklist*
> >> > >>>
> >> > >>> - Easy to do
> >> > >>> - Manual, a bit of reviewer overhead, reviewers needs to know the
> >> > >> process
> >> > >>>
> >> > >>> *Use a bot *
> >> > >>>
> >> > >>> - Automatically add the review questions to each new PR
> >> > >>> - Further details?
> >> > >>>
> >> > >>> *Use GitHub labels*
> >> > >>>
> >> > >>> - Searchable
> >> > >>> - possibly not obvious to new contributors
> >> > >>> - Any restrictions? Do members need to apply at ASF infra to have
> >> > >>> permissions to edit github labels?
> >> > >>
> >> > >>
> >> >
> >> >
> >>
> >
>