mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tianqi Chen <tqc...@cs.washington.edu>
Subject Re: [Discussion] Recognise Reviewers, Besides Committers and PMC
Date Tue, 23 Oct 2018 17:30:19 GMT
>
>  To become a reviewer , he/she would have to have some
> history of code review in MXNet.


In contrary, we should to bring reviewers who have contributed to the repo,
but do not have code review history before, there is always a time gap
between someone who contributed something, to someone who would provide
active high-quality review feedbacks. The reviewer could be an useful
mechanism to encourage the contributors to do so

Tianqi


On Tue, Oct 23, 2018, 8:31 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 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