-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Hi @szha @TaoLv @leezu , with the enhancement (check multiple license header and check *.md files) to 1) files missing a license header
2) multiple license header issue
|
I think all cases fall under contribution in apache so I think we can add apache licenses to them. |
@ciyongch Did you try add the header as follows?
|
Right. Instead of my suggestion of skipping the tests; adding it that way is better. Thanks for that one @leezu |
Hi @ChaiBapchya , @leezu , this enhancement has dependencies as below: |
@mxnet-bot run ci [centos-gpu, centos-cpu, windows-cpu] |
Jenkins CI successfully triggered : [centos-gpu, windows-cpu, centos-cpu] |
The failure CI cases is caused by numpy version changed which is reported in #18600 (master branch), the current header/tool changed has nothing to do with the failure. |
Thanks for driving this. Looks like fixing numpy version helped resolve other pipeline issues [barring windows-cpu] |
Hi @ChaiBapchya , I've updated the numpy version constraint to |
@mxnet-bot run ci [sanity] |
Jenkins CI successfully triggered : [sanity] |
@mxnet-bot run ci [all] |
Jenkins CI successfully triggered : [edge, windows-cpu, windows-gpu, unix-gpu, unix-cpu, centos-gpu, miscellaneous, sanity, clang, website, centos-cpu] |
@mxnet-bot run ci [unix-gpu] |
Jenkins CI successfully triggered : [unix-gpu] |
Hi @szha @leezu @ChaiBapchya , please help to take a review for the new changes/updates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks. LGTM
Shall we cherry-pick this to the master branch too? |
Sure, I will backport the license checker/modification part to both v1.x and master later. |
There seems to be two licenses in cmake/Modules/FindJeMalloc.cmake. Is it because we are skipping 3rdparty folder? Since it's included in the release source, we will need to make sure that all parts we include are checked. |
Hi @szha, the license of cmake/Modules/FindJeMalloc.cmake is already claimed at the top level LICENSE file in https://github.com/apache/incubator-mxnet/blob/v1.7.x/LICENSE#L681-L769, so we can keep it as is now. |
I don't think we discussed the dual license issue for this file and I think it still needs to be discussed even if the original license is declared in the LICENSE file. That said. this is a minor issue and it won't block the release. |
Ok, got it, then let's keep it as is now and try to finalize the fix solution in the next release. |
Yes sounds good. |
@ciyongch is an issue opened for dual licensing/relicensing 3rd party? Is it complete? Can you link that here? Thanks. |
Hi @ChaiBapchya , sorry for missing this, I've just created an issue #19198 for this. |
Description
einsum
related files.Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments