-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[536] Update LICENSE and NOTICE files #541
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
After reviewing this further, I'm wondering whether it's better to add this block to the NOTICE file and leave the LICENSE file unchanged. @zabetak, any thoughts?
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.
the XTable NOTICE should be updated to include the main details from the Avro and Hudi NOTICE files.
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.
the changes in this PR seem unrelated to the xtable-utilities fat jar and seem more related to the XTable source code (and are needed - but the title of this PR seems wrong).
You also need a separate LICENSE and NOTICE for the xtable-utilities fat jar that list all the license and notice details of the classes that you include in that jar. There is a suspicion that some of the classes should not be in there because they have licenses that are not compatible with the Apache license. The OpenJDK JOL classes would seem to me that they should be removed because they are GPL licensed. https://github.com/openjdk/jol/blob/master/LICENSE
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.
@pjfanning Yes these changes are related to XTable source code and updated the PR title as well. There is an in-progress PR to exclude the category X dependencies from xtable-utilities bundle.
#538
For release 0.2.0, we will be excluding the utilities bundle from the release.
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.
Although this PR is merged, I will comment here to keep the discussion in one place.
@jcamachor As a rule of thumb updating the LICENSE file is more frequent than updating the NOTICE file. In most cases we shouldn't touch the NOTICE file.
My understanding after reading https://infra.apache.org/licensing-howto.html#mod-notice is that in this case the NOTICE file does not need to be modified. In fact, we should strive to keep the NOTICE file very simple and "Do not add anything to NOTICE which is not legally required."
Other similar LEGAL tickets (e.g., LEGAL-234) imply that we don't need to duplicate the NOTICE file when we bundle code from other ASF projects.
@pjfanning Can you elaborate a bit on why you believe that the NOTICE file should be copied?
For the LICENSE, file we are in a similar situation. According to https://infra.apache.org/licensing-howto.html#alv2-dep it seems that we don't necessarily need to add anything in the file.
Finally, for the code adapted from Apache Hudi and Apache Spark I would argue that it does not classify as 3rd-party work since the files are developed under the same legal entity (The Apache Software Foundation). These files are already submitted to the ASF by the copyright owner. Moving them around (even across projects) and modifying them does not consist in re-submission.
Anyways, I am not a legal expert but I would say that for the two classes that we adapted from Spark and Hudi we don't need anything in the LICENSE and NOTICE file.
@vinishjail97 Let's converge on this discussion before creating the RC2.
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.
Both too much and too little is problematic and adds burden to consumers. ASF projects copy the one another so we should not allow "bad" patterns to propagate.
Here we are talking about just two files so if there are doubts about what needs to be done let's open a LEGAL ticket.
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.
I'm not against removing the changes to the NOTICE file. The LICENSE changes were the most important part.
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.
I also think that the LICENSE changes are redundant.
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.
create a LEGAL issue then - most ASF projects include details of borrowed code from other ASF projects in their LICENSEs.
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.
https://issues.apache.org/jira/browse/LEGAL-684