mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marco de Abreu <marco.g.ab...@googlemail.com>
Subject Re: Module maintainers proposal
Date Fri, 12 Jan 2018 19:10:50 GMT
Hi Chris,

you have a good point about people being afraid of reviewing PRs which they
are not assigned to and I totally agree that we should encourage everybody
to review PRs.

One important advantage I see in this is the notification: since we are not
using the feature to required an approval, this step is entirely for
information purpose. I, for example, would like to get notified if a PR to
change a CI file would be created. Just as an example: over Christmas, a PR
to update mkl has been pushed without me knowing about it. Somehow, after
my vacation, we started to get issues with mkl test - I only found out
about this PR after quite a long investigation. If we would extend the
usage of the code maintainers, we'll make sure that changes like these will
notify the people who have the best knowledge about that part.

Marco

Am 12.01.2018 8:03 nachm. schrieb "Chris Olivier" <cjolivier01@gmail.com>:

> -1 (binding)
>
> I totally understand the motivation for this (I've definitely saved myself
> some grief by getting called out automatically for CMakeLists.txt stuff,
> for example), but I respectfully decline for the following reason(s):
>
> I feel that defining code-owners has some negative effects.
>
> Other committers may be reluctant to start reviewing and approving PRs
> since they aren't the one listed, so I feel this will in the long-run
> reduce the number of people doing code reviews.
>
> If there aren't enough people doing PR's, then people can complain on dev@
> asking for review.
>
> -Chris
>
>
>
>
> On Fri, Jan 12, 2018 at 10:41 AM, Haibin Lin <haibin@apache.org> wrote:
>
> > +1 (binding)
> >
> > On 2018-01-12 10:10, kellen sunderland <kellen.sunderland@gmail.com>
> > wrote:
> > > +1 (non-binding)
> > >
> > > On Jan 12, 2018 6:32 PM, "Steffen Rochel" <steffenrochel@gmail.com>
> > wrote:
> > >
> > > > I propose to adopt the proposal.
> > > > +1 (non-binding)
> > > >
> > > > Steffen
> > > >
> > > > On Wed, Jan 10, 2018 at 8:39 PM Mu Li <muli.cmu@gmail.com> wrote:
> > > >
> > > > > Hi Isabel,
> > > > >
> > > > > My apologies that not saying that clearly.
> > > > >
> > > > > The purpose of this proposal is encouraging more contributors to
> help
> > > > > review and merge PRs. And also hope to shorten the time for a PR
to
> > be
> > > > > merged. After assigning maintainers to modules, then PR
> contributors
> > can
> > > > > easily contact the reviewers. In other words, github will
> > automatically
> > > > > assign the PR to the maintainer and send a notification email.
> > > > >
> > > > > I don't think I put the term "inbox" in my proposal. I never
> > discussed
> > > > PRs
> > > > > with other contributors by sending email directly, which is less
> > > > effective
> > > > > than just using github. I also don't aware any other contributor
> use
> > the
> > > > > direct email way. So I didn't clarify it on the proposal.
> > > > >
> > > > > On Tue, Jan 9, 2018 at 11:47 AM, Isabel Drost-Fromm <
> > isabel@apache.org>
> > > > > wrote:
> > > > >
> > > > > >
> > > > > >
> > > > > > Am 9. Januar 2018 18:25:50 MEZ schrieb Mu Li <muli.cmu@gmail.com
> >:
> > > > > > >We should encourage to contract a specific contributor for
> issues
> > and
> > > > > > >PRs.
> > > > > >
> > > > > > My head translates "encourage to contact specific contributor"
> into
> > > > > > "encourage to contact specific contributors inbox". This
> translated
> > > > > version
> > > > > > is what I would highly discourage.
> > > > > >
> > > > > > See the disclaimer here for reasons behind that:
> > > > > >
> > > > > > https://home.apache.org/~hossman/#private_q
> > > > > >
> > > > > >
> > > > > > Isabel
> > > > > > --
> > > > > > Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail
> > gesendet.
> > > > > >
> > > > >
> > > >
> > >
> >
>

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