> > 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 <tqc...@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 <tqc...@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 <b...@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 <tqc...@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 <anirudh2...@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 <lanking...@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....@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 < > tqc...@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 < > > >> >>>> steffenroc...@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 < > > >> >> tqc...@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 < > > >> >>>> carinme...@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 < > > >> >>> tqc...@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 > > >> >>>> > > >> >>>> > > >> >>>> > > >> > > >> > > >> > > >
