incubator-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hen <bay...@apache.org>
Subject Re: [VOTE] Apache MXNet (incubating) 1.0.0 release RC0
Date Wed, 29 Nov 2017 21:59:33 GMT
Thanks Justin.

Some comments inline on ones I don't think need fixing; but afaict from
MXNet dev@ activity the plan is to produce a new release and restart the
vote.

On Tue, Nov 28, 2017 at 3:54 PM, Justin Mclean <justin@classsoftware.com>
wrote:

> Hi,
>
> -1 binding due to license, header issues and having a compiled jar in a
> source release.
>
> I checked:
> - incubating in name
> - signatures and hashes correct
> - DISCLAIMER exists
> - LICENSE has issues (see below) I also note that license issues brought
> up last time have not all been addressed. [22]
> - NOTICE seem rather brief considering the number of Apache licensed
> inclusion do any of them have NOTICE files?
>

We can double check, but I know the ones I've looked at didn't use NOTICE
files (the various dmlc/ projects). Plus as these are GitHub submodules, if
they did the NOTICE would automatically come over.


> - A number of source file are missing license headers e.g. [15][16] [18]
> [19] and many others
>

Many of these are not Apache MXNet files but from dependencies. I'll
suggest on dev@ that these submodules be moved into a third-party/
directory. Given the impact of that to builds etc, I'm hoping that can go
in a subsequent release.



> - A number of source look to have had the ASF header incorrectly added.
> - Binary included in source release [20] Note there’s an unresolved legal
> issue about this [21]
>
> Have you run rat on this release it would of help pick up most of these
> issues?
>
> In this file [1] there’s a copyright notice but it also has an ASF header
> which is a little odd. This also occurs in a number of other places.
>

This came up in the previous release (but the opposite way).

When the code came in from MXNet, it had "Copyright Contributors". We
removed that because 'what a useless statement' :), however as there are
400+ contributors and not everyone has signed a CLA, the previous release
vote asked the project to put it back in again, so they did. It's an
eyesore, but we're a bit stuck unless we're prepared to move away from our
rules of "Never remove or move a Copyright statement without approval" and
"Never edit a copyright statement".

We could (I guess) put a comment above it of "# Original MXNet copyright
statement" or some such.


>
> This file [2] also look to incorrectly have an ASF header and it’s unclear
> how the original code is licensed. From a quick like their seems to be many
> files that incorrectly have ASF headers on them e.g. [5][6][7]
> [10][12][13][14] and others.
>
> This file [3] (and others) looks to come from the TVM project which is not
> mentioned in license.
>

Why would it be? We only have to include the LICENSE from TVM, we don't
need to name them.  If TVM want to be identified, they should add a NOTICE
file.


>
> The license for this file [4] is missing from license.
>
> The link for JQuery [8] is missing from the license. Also missing license
> for these files [9][11][17] and probably others.
>
> At this point I gave up so there may be other issues.
>

Understood. A lot of these are systematic characteristics of the project
rather than flat out issues, but definitely some good finds here too.


>
> It also a good idea to publish your keys:
> gpg: assuming signed data in 'apache-mxnet-src-1.0.0.rc0-in
> cubating.tar.gz'
> gpg: Signature made Sat 25 Nov 07:48:02 2017 AEDT
> gpg:                using RSA key 80FD81D7703DF31B
> gpg: requesting key 80FD81D7703DF31B from hkps server
> hkps.pool.sks-keyservers.net
> gpg: Can't check signature: No public key
>
> It’s also a good idea to sign with an apache email address rather than a
> gmail one.
>
> I’m also curious about “CODEOWNERS” file as that doesn’t seem to fit with
> any Apache model I’m aware of.
>

This is a GitHub feature. The name is unfortunate and perhaps we could add
a comment explaining what it is. It allows for automatic addition of
reviewers to a pull request. Kind of like a watch on the code.

The important thing is that a project should never tell a committer they
can't put their name in as a codeowner nee automatic-reviewer.


>
> In “CONTRIBUTORS” there’s a long list of contributors - are their plan to
> make any of these people committers?
>
> Thanks,
> Justin
>
> 1. ./apache-mxnet-src-1.0.0.rc0-incubating/perl-package/AI-MXNe
> t/lib/AI/MXNet.pm
> 2. ./apache-mxnet-src-1.0.0.rc0-incubating/example/image-classi
> fication/predict-cpp/image-classification-predict.cc
> 3. ./apache-mxnet-src-1.0.0.rc0-incubating/nnvm/tvm/src/op/op_util.cc
> 4. ./apache-mxnet-src-1.0.0.rc0-incubating/docs/_static/searcht
> ools_custom.js
> 5. ./apache-mxnet-src-1.0.0.rc0-incubating/src/operator/nn/pool.h
> 6. ./apache-mxnet-src-1.0.0.rc0-incubating/src/operator/contrib
> /nn/deformable_im2col.h
> 7. ./apache-mxnet-src-1.0.0.rc0-incubating/src/operator/contrib
> /psroi_pooling-inl.h
> 8. ./apache-mxnet-src-1.0.0.rc0-incubating/docs/_static/jquery-1.11.1.js
> 9. ./apache-mxnet-src-1.0.0.rc0-incubating/cub/test/mersenne.h
> 10. ./apache-mxnet-src-1.0.0.rc0-incubating/cmake/Modules/FindJe
> Malloc.cmake
> 11.  ./apache-mxnet-src-1.0.0.rc0-incubating/dmlc-core/cmake/Modu
> les/FindCrypto.cmake
> 12. ./apache-mxnet-src-1.0.0.rc0-incubating/example/speech-demo/
> decode_mxnet.sh
> 13. ./apache-mxnet-src-1.0.0.rc0-incubating/example/speech-demo/
> io_func/convert2kaldi.py
> 14. ./apache-mxnet-src-1.0.0.rc0-incubating/src/operator/special
> _functions-inl.h
> 15. apache-mxnet-src-1.0.0.rc0-incubating/example/rnn/bucket_R/rnn.train.R
> 16. apache-mxnet-src-1.0.0.rc0-incubating/tests/travis/r_vignettes.R
> 17. apache-mxnet-src-1.0.0.rc0-incubating/matlab/+mxnet/private/
> parse_json.m
> 18  apache-mxnet-src-1.0.0.rc0-incubating/ps-lite/tests/test_simple_app.cc
> 19. apache-mxnet-src-1.0.0.rc0-incubating/dmlc-core/tracker/yarn
> /src/main/java/org/apache/hadoop/yarn/dmlc/ApplicationMaster.java
> 20. ./apache-mxnet-src-1.0.0.rc0-incubating/nnvm/tvm/apps/androi
> d_rpc/gradle/wrapper/gradle-wrapper.jar
> 21.  https://issues.apache.org/jira/browse/LEGAL-288
> 22. https://github.com/apache/incubator-mxnet/issues/7749
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: general-unsubscribe@incubator.apache.org
> For additional commands, e-mail: general-help@incubator.apache.org
>
>

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