mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Dai <jason....@gmail.com>
Subject Re: [Discussion] Recognise Reviewers, Besides Committers and PMC
Date Fri, 26 Oct 2018 04:34:59 GMT
In general I don’t think it’s a good idea to add more hierarchy for the
project or community. And a relatively easy way to address the issue of
lack of reviewers is to bring in more committers, and focus on one’s code
review contributions (instead of code contributions) when voting someone to
become a committer.

Thanks,
-Jason


On Wed, Oct 24, 2018 at 1:30 AM Tianqi Chen <tqchen@cs.washington.edu>
wrote:

> >
> >  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