You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mxnet.apache.org by Chris Olivier <cj...@gmail.com> on 2017/11/29 20:35:28 UTC

CODEOWNERS file being removed

Per suggestion from Apache, we are removing CODEOWNERS file from root of
mxnet.
If there are any objections, please voice them:

Here are the contents of rht file:

# Owners of Apache MXNet

# Global owners
*        @apache/mxnet-committers

# Owners of language bindings
R-package/*       @thirdwing
scala-package/*       @javelinjs
perl-package/*    @sergeykolychev

# CMake owners
CMakeLists.txt    @cjolivier01
cmake/*          @cjolivier01

Re: CODEOWNERS file being removed

Posted by Chris Olivier <cj...@gmail.com>.
If you can push back on Justin, I can keep it.  Just not looking forward to
going through all of this again just to have it bounced back.

On Wed, Nov 29, 2017 at 1:37 PM, Hen <ba...@apache.org> wrote:

> Was there more discussion than Justin's question about it on general@?
>
> My memory of CODEOWNERS was that it was related to some code review tool,
> but looking at the history of dev@ I only see:
>
> "Can't have changes merged into it until changes to files that have a
> designated code owner <https://help.github.com/articles/about-codeowners/>
> have been approved by that owner"
>
> Reading GitHub:
>
> "Code owners are automatically requested for review when someone opens a
> pull request that modifies code that they own. When someone with admin or
> owner permissions has enabled required reviews
> <https://help.github.com/articles/enabling-required-
> reviews-for-pull-requests/>,
> they also can optionally require approval from a code owner before the
> author can merge a pull request in the repository."
>
> I think that having a notion where anyone can put their name down as an
> automatic reviewer is a good one. Apart from the unfortunate name of the
> file, this should fit with Apache's culture (as long as more than one
> committer can, and as long as we never say a committer can't decide to put
> themselves as a codeowner). A comment in the file explaining this would be
> good.
>
> We should however, never enable the code-owner option of required reviews.
> As that's an Admin/Owner feature, and I don't think Apache Infra would do
> that, I think we're good there.
>
> So my suggestion would be to add a comment that this is an
> automatically-listed-on-reviews tool.
>
> Hen
>
>
> On Wed, Nov 29, 2017 at 12:35 PM, Chris Olivier <cj...@gmail.com>
> wrote:
>
> > Per suggestion from Apache, we are removing CODEOWNERS file from root of
> > mxnet.
> > If there are any objections, please voice them:
> >
> > Here are the contents of rht file:
> >
> > # Owners of Apache MXNet
> >
> > # Global owners
> > *        @apache/mxnet-committers
> >
> > # Owners of language bindings
> > R-package/*       @thirdwing
> > scala-package/*       @javelinjs
> > perl-package/*    @sergeykolychev
> >
> > # CMake owners
> > CMakeLists.txt    @cjolivier01
> > cmake/*          @cjolivier01
> >
>

Re: CODEOWNERS file being removed

Posted by Chris Olivier <cj...@gmail.com>.
ok

On Wed, Nov 29, 2017 at 1:42 PM, Tianqi Chen <tq...@cs.washington.edu>
wrote:

> Codeowner feature is used to automatically trigger the review request for
> the people. All the committers are already listed as global owners, which
> is consistent with Apache's policy
>
> Tianqi
>
> On Wed, Nov 29, 2017 at 1:37 PM, Hen <ba...@apache.org> wrote:
>
> > Was there more discussion than Justin's question about it on general@?
> >
> > My memory of CODEOWNERS was that it was related to some code review tool,
> > but looking at the history of dev@ I only see:
> >
> > "Can't have changes merged into it until changes to files that have a
> > designated code owner <https://help.github.com/
> articles/about-codeowners/>
> > have been approved by that owner"
> >
> > Reading GitHub:
> >
> > "Code owners are automatically requested for review when someone opens a
> > pull request that modifies code that they own. When someone with admin or
> > owner permissions has enabled required reviews
> > <https://help.github.com/articles/enabling-required-
> > reviews-for-pull-requests/>,
> > they also can optionally require approval from a code owner before the
> > author can merge a pull request in the repository."
> >
> > I think that having a notion where anyone can put their name down as an
> > automatic reviewer is a good one. Apart from the unfortunate name of the
> > file, this should fit with Apache's culture (as long as more than one
> > committer can, and as long as we never say a committer can't decide to
> put
> > themselves as a codeowner). A comment in the file explaining this would
> be
> > good.
> >
> > We should however, never enable the code-owner option of required
> reviews.
> > As that's an Admin/Owner feature, and I don't think Apache Infra would do
> > that, I think we're good there.
> >
> > So my suggestion would be to add a comment that this is an
> > automatically-listed-on-reviews tool.
> >
> > Hen
> >
> >
> > On Wed, Nov 29, 2017 at 12:35 PM, Chris Olivier <cj...@gmail.com>
> > wrote:
> >
> > > Per suggestion from Apache, we are removing CODEOWNERS file from root
> of
> > > mxnet.
> > > If there are any objections, please voice them:
> > >
> > > Here are the contents of rht file:
> > >
> > > # Owners of Apache MXNet
> > >
> > > # Global owners
> > > *        @apache/mxnet-committers
> > >
> > > # Owners of language bindings
> > > R-package/*       @thirdwing
> > > scala-package/*       @javelinjs
> > > perl-package/*    @sergeykolychev
> > >
> > > # CMake owners
> > > CMakeLists.txt    @cjolivier01
> > > cmake/*          @cjolivier01
> > >
> >
>

Re: CODEOWNERS file being removed

Posted by Tianqi Chen <tq...@cs.washington.edu>.
Codeowner feature is used to automatically trigger the review request for
the people. All the committers are already listed as global owners, which
is consistent with Apache's policy

Tianqi

On Wed, Nov 29, 2017 at 1:37 PM, Hen <ba...@apache.org> wrote:

> Was there more discussion than Justin's question about it on general@?
>
> My memory of CODEOWNERS was that it was related to some code review tool,
> but looking at the history of dev@ I only see:
>
> "Can't have changes merged into it until changes to files that have a
> designated code owner <https://help.github.com/articles/about-codeowners/>
> have been approved by that owner"
>
> Reading GitHub:
>
> "Code owners are automatically requested for review when someone opens a
> pull request that modifies code that they own. When someone with admin or
> owner permissions has enabled required reviews
> <https://help.github.com/articles/enabling-required-
> reviews-for-pull-requests/>,
> they also can optionally require approval from a code owner before the
> author can merge a pull request in the repository."
>
> I think that having a notion where anyone can put their name down as an
> automatic reviewer is a good one. Apart from the unfortunate name of the
> file, this should fit with Apache's culture (as long as more than one
> committer can, and as long as we never say a committer can't decide to put
> themselves as a codeowner). A comment in the file explaining this would be
> good.
>
> We should however, never enable the code-owner option of required reviews.
> As that's an Admin/Owner feature, and I don't think Apache Infra would do
> that, I think we're good there.
>
> So my suggestion would be to add a comment that this is an
> automatically-listed-on-reviews tool.
>
> Hen
>
>
> On Wed, Nov 29, 2017 at 12:35 PM, Chris Olivier <cj...@gmail.com>
> wrote:
>
> > Per suggestion from Apache, we are removing CODEOWNERS file from root of
> > mxnet.
> > If there are any objections, please voice them:
> >
> > Here are the contents of rht file:
> >
> > # Owners of Apache MXNet
> >
> > # Global owners
> > *        @apache/mxnet-committers
> >
> > # Owners of language bindings
> > R-package/*       @thirdwing
> > scala-package/*       @javelinjs
> > perl-package/*    @sergeykolychev
> >
> > # CMake owners
> > CMakeLists.txt    @cjolivier01
> > cmake/*          @cjolivier01
> >
>

Re: CODEOWNERS file being removed

Posted by Hen <ba...@apache.org>.
Was there more discussion than Justin's question about it on general@?

My memory of CODEOWNERS was that it was related to some code review tool,
but looking at the history of dev@ I only see:

"Can't have changes merged into it until changes to files that have a
designated code owner <https://help.github.com/articles/about-codeowners/>
have been approved by that owner"

Reading GitHub:

"Code owners are automatically requested for review when someone opens a
pull request that modifies code that they own. When someone with admin or
owner permissions has enabled required reviews
<https://help.github.com/articles/enabling-required-reviews-for-pull-requests/>,
they also can optionally require approval from a code owner before the
author can merge a pull request in the repository."

I think that having a notion where anyone can put their name down as an
automatic reviewer is a good one. Apart from the unfortunate name of the
file, this should fit with Apache's culture (as long as more than one
committer can, and as long as we never say a committer can't decide to put
themselves as a codeowner). A comment in the file explaining this would be
good.

We should however, never enable the code-owner option of required reviews.
As that's an Admin/Owner feature, and I don't think Apache Infra would do
that, I think we're good there.

So my suggestion would be to add a comment that this is an
automatically-listed-on-reviews tool.

Hen


On Wed, Nov 29, 2017 at 12:35 PM, Chris Olivier <cj...@gmail.com>
wrote:

> Per suggestion from Apache, we are removing CODEOWNERS file from root of
> mxnet.
> If there are any objections, please voice them:
>
> Here are the contents of rht file:
>
> # Owners of Apache MXNet
>
> # Global owners
> *        @apache/mxnet-committers
>
> # Owners of language bindings
> R-package/*       @thirdwing
> scala-package/*       @javelinjs
> perl-package/*    @sergeykolychev
>
> # CMake owners
> CMakeLists.txt    @cjolivier01
> cmake/*          @cjolivier01
>