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 <jus...@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-unsubscr...@incubator.apache.org > For additional commands, e-mail: general-h...@incubator.apache.org > >
