Thank you Dave, Sergio and Henri for your comments. I will make sure to follow the points specified by you in future emails.
Sergio, In reference to the two points you mentioned for your negative vote, I would like to add this - 1. Tianqi Chen has confirmed (via slack) that for this binary ' nnvm/tvm/apps/android_rpc/gradle/wrapper/gradle-wrapper.jar’ there isn't a notice file that should be added. 2. The addition of the copyright line that you mentioned was discussed in the 0.11.0 vote thread. In order to track these comments from 0.11.0 release, I created this github issue — https://github.com/apache/incubator-mxnet/issues/7749 And this Issue was fixed for the 1.0.0 release in this PR — https://github.com/apache/incubator-mxnet/pull/8688 However, if you still believe that we are missing something, I can make changes immediately after this release to fix any gaps. Currently, the goal is to have MXNet version 1.0.0 release ready before the NIPS conference that starts on 04-Dec, 2017 and it would be helpful if you would be ok to change your vote to a positive one considering these points. We will definitely fix any pending issues immediately after the release. Thanks, Meghna Baijal On Tue, Nov 28, 2017 at 1:09 AM, Hen <[email protected]> wrote: > Thank you for the review Sergio :) > > On Mon, Nov 27, 2017 at 7:37 PM, Sergio Fernández <[email protected]> > wrote: > > > Hi, > > > > * Incubator DISCLAIMER included. > > * LICENSE file contains information that should go in the NOTICE. > > > > Interested in which you think should be in there. The license file is > pointing to licenses lower in the tree that contain their various items. > One could argue the Copyright for the BSD etc should either be in LICENSE > or NOTICE, but it is in those lower directories. > > > > * Build worked fine on my desktop (Ubuntu 17.10, GCC 7.2.0). > > > > * I'd put the install instruction somewhere more prominent > > than docs/install/index.md, and probably more CLI-friendly text > document. > > Actually I was confused by the very different instruction from > > http://mxnet.apache.org/install/index.html So always take into account > > usability of your source release. > > > > This one always bothers me too. I get why there are lots of .md files, > that's how life on GitHub works, but it makes life difficult when reviewing > the source tarball. > > > > * The KEYS file are not correctly linked in the VOTE email. They weren't > > that hard to find at https://dist.apache.org/repos/dist/dev/incubator/ > > mxnet/KEYS, but should be properly linked.. > > > > Event thoughmy vote for this RC1 is -1 (binding), because: > > > > * Binary nnvm/tvm/apps/android_rpc/gradle/wrapper/gradle-wrapper.jar is > > distributed without NOTICE. > > > > Noting that nnvm is a third party app (though one that some of the MXNet > committers work on). Perhaps a bug report/patch could be submitted there. > > However, the gradle wrapper is under Apache 2.0, and does not have a NOTICE > file (uppercase NOTICE). NNVM has an Apache 2.0 license in its root. It's > not as user friendly as it could be, but I believe it is being distributed > with license notice (lower case notice), which is all the license requires. > > (While Gradle does have additional licensing in their LICENSE file, I don't > see any of that in the gradle-wrapper.jar). > > > > * Files' header contain, after the normative ASF license header, > confusing > > copyright information that should be cleaned to avoid confusions. > > > > If you mean: > > * Copyright (c) 2017 by Contributors > > Then that was reintroduced by specific request of a previous Incubator > vote/Incubator PMC feedback. Not all the code is covered by a CLA/grant (I > counted 400 contributors), so the original Copyright statement is > maintained (the enormously ugly 'Copyright Contributors'). > > Thanks, > > Hen >
