Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

License fixes #8873

Merged
merged 18 commits into from
Nov 30, 2017
Merged

License fixes #8873

merged 18 commits into from
Nov 30, 2017

Conversation

cjolivier01
Copy link
Member

Description

(Brief description on what this PR is about)

Checklist

Essentials

  • Passed code style checking (make lint)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • For user-facing API changes, API doc string has been updated. For new C++ functions in header files, their functionalities and arguments are well-documented.
  • To my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@piiswrong
Copy link
Contributor

mshadow and nnvm are not apache projects. They don't need this header

@cjolivier01
Copy link
Member Author

So which ones do? We got a downvote by Apache on the release due to problems with licensing headers in files in these modules.

@cjolivier01
Copy link
Member Author

cjolivier01 commented Nov 29, 2017

It would help if some dmlc people got on general@ and dispute these issues rather than just Apache committers try to make a case for dmlc people's wishes and restrictions.

@piiswrong
Copy link
Contributor

We should solve this by not including any 3rdparty source code in the release tar.
For example we can't add asf header to nvdia cub

* specific language governing permissions and limitations
* under the License.
*/

/*
* searchtools.js_t
* ~~~~~~~~~~~~~~~~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During 0.11.0 release there was a comment to remove the asf header from this file and I did this here - #8247

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, ok removing

@mbaijal
Copy link
Contributor

mbaijal commented Nov 29, 2017

Looks ok to me.
The submodule changes are not being merged, right?

@cjolivier01
Copy link
Member Author

Right, no submodule changes are being merged.

@cjolivier01
Copy link
Member Author

As suggested, I will move cub to 3rdparty in another PR

@cjolivier01 cjolivier01 merged commit 4306d43 into apache:v1.0.0 Nov 30, 2017
@cjolivier01 cjolivier01 deleted the v1.0.0.rc1 branch November 30, 2017 00:17
@@ -234,7 +237,7 @@
1. Fast R-CNN - For details, see example/rcnn/LICENSE
2. Faster R-CNN - For details, see example/rcnn/LICENSE
3. tree_lstm - For details, see example/gluon/tree_lstm/LICENSE

4. JQuery - For details, see http://jquery.org/license

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should include the actual license text in the package, since it could vary between releases of jquery. Use the same pointer file structure as other files mentioned in this section.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants