flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Rohrmann <trohrm...@apache.org>
Subject Re: [DISCUSS] (Not) tagging reviewers
Date Tue, 24 Jan 2017 13:06:48 GMT
I agree with Aljoscha that tagging the PRs helps me to get notified about
PRs where I could be of help. But I also think that it should not be the
ultimate responsibility of the tagged person(s) to review the code.
Everyone should feel empowered to do so. And also the tagging does not free
oneself from browsing the PR list to check out the open PRs.

@Theo, usually this notification should already happen via the JIRA
integration given that the person who created the JIRA issue also watches
it.

Cheers,
Till

On Tue, Jan 24, 2017 at 1:29 PM, Theodore Vasiloudis <
theodoros.vasiloudis@gmail.com> wrote:

> I was wondering how this relates to the shepherding of PRs we have
> discussed in the past. If I make a PR for an issue reported from a specific
> committer, doesn't tagging them make sense?
>
> Has the shepherding of PRs been tried out?
>
> On Tue, Jan 24, 2017 at 12:17 PM, Aljoscha Krettek <aljoscha@apache.org>
> wrote:
>
> > It seems I'm in a bit of a minority here but I like the @R tags. There
> are
> > simply to many pull request for someone to keep track of all of them and
> if
> > someone things that a certain person would be good for reviewing a change
> > then tagging them helps them notice the PR.
> >
> > I think the tag should not mean that only that person can/should review
> the
> > PR, it should serve as a proposal.
> >
> > I'm happy to not use it anymore if everyone else doesn't like them.
> >
> > On Sat, 21 Jan 2017 at 00:53 Fabian Hueske <fhueske@gmail.com> wrote:
> >
> > > Hi Haohui,
> > >
> > > reviewing pull requests is a great way of contributing to the
> community!
> > >
> > > I am not aware of specific instructions for the review process. The are
> > > some dos and don'ts on our "contribute code" page [1] that should be
> > > considered. Apart from that, I think the best way to start is to become
> > > familiar with a certain part of the code base (reading code,
> > contributing)
> > > and then to look out for pull requests that address the part you are
> > > familiar with.
> > >
> > > The review does not have to cover all aspects of a PR (a committer will
> > > have a look as well), but from my personal experience the effort to
> > review
> > > a PR is often much lower if some other person has had a look at it
> > already
> > > and gave feedback.
> > > I think this can help a lot to reduce the review "load" on the
> > committers.
> > > Maybe you find some contributors who are interested in the same
> > components
> > > as you and you can start reviewing each others code.
> > >
> > > Thanks,
> > > Fabian
> > >
> > > [1] http://flink.apache.org/contribute-code.html#coding-guidelines
> > >
> > >
> > > 2017-01-20 23:02 GMT+01:00 jincheng sun <sunjincheng121@gmail.com>:
> > >
> > > > I totally agree with all of your ideas.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > Best wishes,
> > > >
> > > >
> > > >
> > > > SunJincheng.
> > > >
> > > > Stephan Ewen <sewen@apache.org>于2017年1月16日 周一19:42写道:
> > > >
> > > > > Hi!
> > > > >
> > > > >
> > > > >
> > > > > I have seen that recently many pull requests designate reviews by
> > > writing
> > > > >
> > > > > "@personA review please" or so.
> > > > >
> > > > >
> > > > >
> > > > > I am personally quite strongly against that, I think it hurts the
> > > > community
> > > > >
> > > > > work:
> > > > >
> > > > >
> > > > >
> > > > >   - The same few people get usually "designated" and will typically
> > get
> > > > >
> > > > > overloaded and often not do the review.
> > > > >
> > > > >
> > > > >
> > > > >   - At the same time, this discourages other community members from
> > > > looking
> > > > >
> > > > > at the pull request, which is totally undesirable.
> > > > >
> > > > >
> > > > >
> > > > >   - In general, review participation should be "pull based" (person
> > > > decides
> > > > >
> > > > > what they want to work on) not "push based" (random person pushes
> > work
> > > to
> > > > >
> > > > > another person). Push-based just creates the wrong feeling in a
> > > community
> > > > >
> > > > > of volunteers.
> > > > >
> > > > >
> > > > >
> > > > >   - In many cases the designated reviews are not the ones most
> > > > >
> > > > > knowledgeable in the code, which is understandable, because how
> > should
> > > > >
> > > > > contributors know whom to tag?
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Long story short, why don't we just drop that habit?
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Greetings,
> > > > >
> > > > > Stephan
> > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message