Skip to content
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

Correct copyright notices to reflect Copyright OpenSearch Contributors #712

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

tmarkley
Copy link
Contributor

@tmarkley tmarkley commented Aug 7, 2021

Description

Removed Amazon copyright, the correct copyright for OpenSearch projects is "Copyright OpenSearch Contributors".

Signed-off-by: Tommy Markley [email protected]

Issues Resolved

Resolves #711

Check List

  • Commits are signed per the DCO using --signoff

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 9ead91a

@kavilla
Copy link
Member

kavilla commented Aug 7, 2021

Did we need to do something like this #350 as well ?

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

You also need to remove the year from Copyright 2021 OpenSearch Contributors in NOTICE.

boktorbb
boktorbb previously approved these changes Aug 11, 2021
seanneumann
seanneumann previously approved these changes Aug 31, 2021
Copy link
Contributor

@seanneumann seanneumann left a comment

Choose a reason for hiding this comment

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

LGTM

@tmarkley
Copy link
Contributor Author

I'm fixing the NOTICE generator and sending out a revision.

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed cae70ae

kavilla
kavilla previously approved these changes Aug 31, 2021
NOTICE.txt Outdated
OpenSearch
Copyright 2021 OpenSearch Contributors
This product includes software, including Kibana source code, developed by Elasticsearch (http://www.elastic.co).
Copyright 2012-2021 Elasticsearch B.V.
Copy link
Member

Choose a reason for hiding this comment

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

Was this in the original Kibana license? I think we should then reproduce it as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Part of this was, yes. Do we need to include their NOTICE header? https://github.com/elastic/kibana/blob/v7.10.2/NOTICE.txt

Copy link
Member

Choose a reason for hiding this comment

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

I am not 100% sure, let's ask @hyandell?

Copy link

Choose a reason for hiding this comment

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

We need to include the relevant NOTICE text for any Apache-2.0 with a NOTICE content that we bring in. Make sure it's the NOTICE text at that time; i.e. don't include that project's NOTICE 2 years later on just because it's the latest on their main branch.

The only proviso is if we have removed the content it refers to.

In the case of this NOTICE file PR in general, my concerns are:

  • Removing our copyright year: Not hugely concerned. We don't usually put copyright years on our OSS, but we included it in this case to avoid it seeming odd when the original codebase did use years.
  • Changing the top "This product includes" language. I'm not seeing a great need to do this.
  • Changing Elastic's copyright date. This is concerning and should only be done if you're identifying that nothing post 2018 was included in the repository.
  • Adding the line about Joda. Is this true for this project, or is this being copied from another project?
  • Removing other items in the NOTICE. This should only be done if you've conducted a review to confirm they're no longer there. I think we should do that one GitHub issue a time, with a PR for each, so that we are properly tracking the review we've done to confirm that that notice-element no longer, or never did, apply.

Copy link
Member

Choose a reason for hiding this comment

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

We cleared the whole removing year for our copyright and the language earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Henri for the clarifications. I'm deferring the NOTICE changes for now since they're not required and I've opened #765 to address discrepancies that exist between the current NOTICE file and the generator script.

NOTICE.txt Outdated
@@ -145,70 +150,6 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.

---
Copy link
Member

Choose a reason for hiding this comment

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

Are these part of the codebase. Do not remove notices if they are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is auto-generated, so I believe they have been removed from the codebase.

NOTICE.txt Outdated
@@ -115,10 +124,6 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.

---
Copy link
Member

Choose a reason for hiding this comment

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

Are these still part of the codebase. Do not remove notices if they are.

@tmarkley tmarkley requested a review from hyandell September 2, 2021 18:16
ananzh
ananzh previously approved these changes Sep 2, 2021
@tmarkley
Copy link
Contributor Author

tmarkley commented Sep 2, 2021

I've removed the changes to the NOTICE file so that we can discuss them in a different thread. It's clear to me that there's a discrepancy between the current NOTICE file in the repo and the generated NOTICE file, so I've documented that here: #765

This PR now just includes the required change in the README.

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed cf10116

@tmarkley tmarkley added the docs Improvements or additions to documentation label Sep 2, 2021
@tmarkley tmarkley merged commit 64b3312 into opensearch-project:main Sep 7, 2021
tmarkley pushed a commit to tmarkley/OpenSearch-Dashboards that referenced this pull request Sep 7, 2021
kavilla pushed a commit that referenced this pull request Sep 7, 2021
@tmarkley tmarkley deleted the 711 branch September 10, 2021 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation v1.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Correct copyright notices to reflect Copyright OpenSearch Contributors.
8 participants