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" <[email protected]>:

> -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 <[email protected]> wrote:
>
> > +1 (binding)
> >
> > On 2018-01-12 10:10, kellen sunderland <[email protected]>
> > wrote:
> > > +1 (non-binding)
> > >
> > > On Jan 12, 2018 6:32 PM, "Steffen Rochel" <[email protected]>
> > wrote:
> > >
> > > > I propose to adopt the proposal.
> > > > +1 (non-binding)
> > > >
> > > > Steffen
> > > >
> > > > On Wed, Jan 10, 2018 at 8:39 PM Mu Li <[email protected]> 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 <
> > [email protected]>
> > > > > wrote:
> > > > >
> > > > > >
> > > > > >
> > > > > > Am 9. Januar 2018 18:25:50 MEZ schrieb Mu Li <[email protected]
> >:
> > > > > > >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.
> > > > > >
> > > > >
> > > >
> > >
> >
>

Reply via email to