mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tianqi Chen <tqc...@apache.org>
Subject Re: [Discussion] Recognise Reviewers, Besides Committers and PMC
Date Tue, 23 Oct 2018 15:31:08 GMT
It is true that is a person is trusted to review code they should be
considered as a committer.

On the other hand, it is also super valuable to solicit reviews from
contributors who have the potential to become committers. These code
reviews may not necessarily directly grant the merge of the code, they are
super helpful to the one who sends the patch, the committer who would merge
the code, and the reviewer(since they can learn from code review of
committers).

There is always a process for a contributor to learn, and starting by
contributing code reviews is a good part. Potential committers don't simply
show up in the community. We need to find them, encourage them to
contribute and provide mentorship. I am proposing to recognize them and
list them as reviewers, so committers can solicit reviews and watch these
process. This would in general results in more reviews for each patch, and
more evidence to help us to find potential committers

Tianqi

On Tue, Oct 23, 2018 at 8:29 AM Tianqi Chen <tqchen@apache.org> wrote:

> It is true that is a person is trusted to review code they should be
> considered as a reviewer.
>
> But what I am trying to say is that -- it is also super valuable to
> solicit reviews from contributors who have the potential to become
> committers. These code reviews may not necessarily directly grant the merge
> of the code, they are super helpful to the one who sends the patch, the
> committer who would merge the code, and the reviewer(since they can learn
> from code review of committers).
>
> There is always a process for a contributor to learn, and starting by
> contributing code reviews is a good part. I am proposing to recognize them
> and list them as reviewers, so committers can solicit reviews and watch
> these process. This would in general results in more reviews for each
> patch, and more evidence to help us to find potential committers
>
> Tianqi
>
> On Tue, Oct 23, 2018 at 7:08 AM Bob Paulin <bob@bobpaulin.com> wrote:
>
>> Generally if a person is trusted to review code they should be
>> considered as a committer.  Agree that having good reviewers are
>> important but the way most projects recognize those efforts is by making
>> them committers.
>>
>>
>> - Bob
>>
>>
>> On 10/22/2018 5:23 PM, Chris Olivier wrote:
>> > Are there any other major Apache projects which have this designation?
>> I
>> > am always continually suspicious of efforts to reinvent Apache rules
>> from
>> > other non-Apache projects, when Apache projects have historically been
>> > quite successful within the Apache platform.  In fact, operating
>> outside of
>> > Apache norms is already a major problem as everyone knows.  We are only
>> > just now splitting Committer/PMC into two separate groups. Splitting
>> into
>> > three seems a bit much at this juncture unless there's some good
>> precedents.
>> >
>> >
>> >
>> >
>> > On Mon, Oct 22, 2018 at 2:17 PM Tianqi Chen <tqchen@apache.org> wrote:
>> >
>> >> The situation most projects are facing(including us), is lack of code
>> >> reviews. Code reviews are the most important part of the project, and
>> >> high-quality reviews are extremely time-consuming, maybe as much as so
>> >> as the code itself. Usually, it is only committers do the code
>> reviews, the
>> >> code reviews from committers are important, as they are the serve as
>> >> the gate-keeper of the quality of the code.  In my experience, I
>> >> usually find the reviews from non-committer super helpful, and they
>> >> help the committer to catch problems that are otherwise overlooked.
>> >>
>> >> However, it is very hard to get contributors to do code reviews unless
>> we
>> >> solicit them. It is definitely harder than getting code
>> contributions.  The
>> >> Reviewer mechanism could provide a way to do so. We can recognize
>> >> contributors, bring them as reviewers and encourage them to do the code
>> >> reviews by explicitly soliciting. The reviewers can learn from the
>> >> committer reviews,
>> >> which serves as a role model for what is being expected. Naturally,
>> this
>> >> likely helps us find more good reviewers and bought them committer.
>> >>
>> >> Cheers
>> >> Tianqi
>> >>
>> >> On Mon, Oct 22, 2018 at 1:09 PM Anirudh <anirudh2290@gmail.com> wrote:
>> >>
>> >>> -1. I dont see the need for additional level of hierarchy. I totally
>> am
>> >> for
>> >>> recognizing good code reviewers. We can recognize this by making them
>> >>> committers. Being a good reviewer should be sufficient to become a
>> >>> committer in my opinion. (Assuming that there is a seperation between
>> >> PPMC
>> >>> and committers).
>> >>>
>> >>> Anirudh
>> >>>
>> >>> On Mon, Oct 22, 2018 at 8:28 AM Qing Lan <lanking520@live.com>
wrote:
>> >>>
>> >>>> +1
>> >>>> Let's have a reviewer list somewhere with a certain format: such
as
>> >> C++,
>> >>>> Gluon, Scala/Java based on language or some other category. etc.
In
>> the
>> >>>> future, label bot would automatically assign reviewers based on
this
>> >> kind
>> >>>> of documentation.
>> >>>>
>> >>>> Thanks,
>> >>>> Qing
>> >>>>
>> >>>> ´╗┐On 10/21/18, 11:44 PM, "YiZhi Liu" <eazhi.liu@gmail.com>
wrote:
>> >>>>
>> >>>>     +1
>> >>>>     I also suggest add reviewer list link to the PR template, so
that
>> >>>>     developers can easily request review from those reviewers.
>> >>>>     On Sun, Oct 21, 2018 at 8:30 PM Tianqi Chen <tqchen@apache.org>
>> >>> wrote:
>> >>>>     >
>> >>>>     > I was suggesting something more concrete:
>> >>>>     >
>> >>>>     > - Add a Reviewers section to
>> >>>>     >
>> >>>>
>> https://github.com/apache/incubator-mxnet/blob/master/CONTRIBUTORS.md
>> >> to
>> >>>>     > list a list of Reviewers.
>> >>>>     >     - This is a "pesudo role", but holds weight as committers
>> >>> should
>> >>>> highly
>> >>>>     > value their reviews during the PR process.
>> >>>>     > - The committers/PMC could actively look for good contributors
>> >> and
>> >>>> nominate
>> >>>>     > them as Reviewer.
>> >>>>     > - Contributors are encouraged to seek reviews from the
list of
>> >>>> reviewers.
>> >>>>     > - The committers should actively solicit code reviews from
the
>> >>>> reviewers
>> >>>>     > when reviewing PRs and take their reviews into serious
>> >>> consideration.
>> >>>>     >
>> >>>>     > - PMCs should actively look for new committers in the current
>> >>>> Reviewers
>> >>>>     >    - Notably, the history reviews plus contribution likely
will
>> >>>> provide a
>> >>>>     > good indication on whether the person can uphold the quality
>> >>>> standard of
>> >>>>     > the codebase, and provide helpful feedbacks(which is the
trait
>> >> that
>> >>>> needed
>> >>>>     > from committer to merge code)
>> >>>>     >
>> >>>>     > Tianqi
>> >>>>     >
>> >>>>     >
>> >>>>     > On Sun, Oct 21, 2018 at 5:13 PM Steffen Rochel <
>> >>>> steffenrochel@gmail.com>
>> >>>>     > wrote:
>> >>>>     >
>> >>>>     > > +1
>> >>>>     > > With the release announcement for MXNet 1.3 all contributors
>> >>> incl.
>> >>>> code
>> >>>>     > > reviewers have been recognized. I suggest all future
release
>> >>>> announcements
>> >>>>     > > should include such recognition. Are you suggesting
to
>> >> highlight
>> >>>> most
>> >>>>     > > active reviewers in release announcement or regularly
(e.g.
>> >>>> monthly),
>> >>>>     > > specifically from non-committers?
>> >>>>     > >
>> >>>>     > > On Sun, Oct 21, 2018 at 10:11 AM Tianqi Chen <
>> >> tqchen@apache.org>
>> >>>> wrote:
>> >>>>     > >
>> >>>>     > > > Also re another email-thread(I sent out one with
my
>> >>>> institutional email
>> >>>>     > > > which get blocked initially, so this one was
a bit
>> >> duplication
>> >>>> of that).
>> >>>>     > > I
>> >>>>     > > > think it should really be the job of committers
to
>> recognize
>> >>>> potential
>> >>>>     > > > reviewers, github also makes it easier to do
so, e.g.
>> >>>>     > > >
>> >>>>     > > >
>> >>>>     > >
>> >>>>
>> >>
>> https://github.com/apache/incubator-mxnet/pulls?utf8=%E2%9C%93&q=reviewed-by%3Apiiswrong
>> >>>>     > > >
>> >>>>     > > > Tianqi
>> >>>>     > > >
>> >>>>     > > > On Fri, Oct 19, 2018 at 12:05 PM Carin Meier
<
>> >>>> carinmeier@gmail.com>
>> >>>>     > > wrote:
>> >>>>     > > >
>> >>>>     > > > > +1 Great idea. Adding a name to the contributor
list is a
>> >>> good
>> >>>> idea.
>> >>>>     > > > Also,
>> >>>>     > > > > I've found that thanking the person for
the review on the
>> >> PR
>> >>>> is another
>> >>>>     > > > way
>> >>>>     > > > > to express gratitude for their time and
effort.
>> >>>>     > > > >
>> >>>>     > > > > On Fri, Oct 19, 2018 at 2:51 PM Tianqi Chen
<
>> >>> tqchen@apache.org>
>> >>>> wrote:
>> >>>>     > > > >
>> >>>>     > > > > > Dear MXNet Community:
>> >>>>     > > > > >
>> >>>>     > > > > > There is a great discussion going on
in terms of
>> lowering
>> >>>> the barrier
>> >>>>     > > > of
>> >>>>     > > > > > entries and encourage more contribution
to the project.
>> >>> One
>> >>>> of the
>> >>>>     > > > > general
>> >>>>     > > > > > goals is to encourage a broader pool
of contributions.
>> I
>> >>>> want to make
>> >>>>     > > > the
>> >>>>     > > > > > following proposal:
>> >>>>     > > > > >
>> >>>>     > > > > > Besides Committers and PMC, let us
also recognize
>> >> Reviewers
>> >>>> in the
>> >>>>     > > > > > community.  This is a "pseudo role"
as there is no such
>> >>>> official role
>> >>>>     > > > in
>> >>>>     > > > > > Apache. But I want to explore the possibility
of
>> >>> recognizing
>> >>>> active
>> >>>>     > > > > > reviewers for example, by adding a
list of names in the
>> >>>> contributor
>> >>>>     > > > list.
>> >>>>     > > > > > In general, I find it is really helpful
to have more
>> code
>> >>>> reviews.
>> >>>>     > > > > > Recognizing good reviewers early enables
us to find
>> >>> committer
>> >>>>     > > > candidates,
>> >>>>     > > > > > and encourage them to contribute and
understand what is
>> >> the
>> >>>> bar of
>> >>>>     > > code
>> >>>>     > > > > > quality that is required to merge the
code.
>> >>>>     > > > > >
>> >>>>     > > > > > This can provide the community with
more evidence when
>> >>>> recruiting new
>> >>>>     > > > > > committers. After all the write access
of committership
>> >> is
>> >>>> about to
>> >>>>     > > the
>> >>>>     > > > > > code and understand the consequence
of the
>> responsibility
>> >>> --
>> >>>> which is
>> >>>>     > > > > > usually can be found in high-quality
review history.
>> >>>>     > > > > >
>> >>>>     > > > > > Please let me know what you think.
>> >>>>     > > > > >
>> >>>>     > > > > > Tianqi
>> >>>>     > > > > >
>> >>>>     > > > >
>> >>>>     > > >
>> >>>>     > >
>> >>>>
>> >>>>
>> >>>>
>> >>>>     --
>> >>>>     Yizhi Liu
>> >>>>     DMLC member
>> >>>>     Amazon Web Services
>> >>>>     Vancouver, Canada
>> >>>>
>> >>>>
>> >>>>
>>
>>
>>

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