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

Add noticefile generator and update NOTICE.txt #20

Merged
merged 6 commits into from
May 21, 2024

Conversation

navarone-feekery
Copy link
Collaborator

Adds a generator script that builds a NOTICE.txt file based on project dependencies.

The script will attempt to fetch SPDX license information for ruby gems. If this can't be found, it will fetch the LICENSE file directly, and failing that it will require the file to be manually added to the repository.

Run with make notice.

@navarone-feekery navarone-feekery requested a review from a team May 15, 2024 10:29
@navarone-feekery navarone-feekery changed the title Navarone/add noticefile generator Add noticefile generator and update NOTICE.txt May 15, 2024
Copy link
Member

@artem-shelkovnikov artem-shelkovnikov left a comment

Choose a reason for hiding this comment

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

Just checking - did you check with Open Source team the format of NOTICE.txt?

@@ -0,0 +1,23 @@
Portions copyright (c) 2010 Andre Arko
Copy link
Member

Choose a reason for hiding this comment

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

Also - do you wanna add those to gitignore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes a lot of sense actually

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@artem-shelkovnikov I've added the downloadable license files to gitignore. The manual ones are still there as otherwise anyone else running make notice would need to go find these licenses themselves.

@navarone-feekery
Copy link
Collaborator Author

Just checking - did you check with Open Source team the format of NOTICE.txt?

Good question; no I did not. I'll confirm before merging.

@seanstory
Copy link
Member

did you check with Open Source team the format of NOTICE.txt?

My understanding is that there's not a strict format, it's meant for human consumption, so a "best effort" at clarity is sufficient.

Since this is the same format we've been shipping with the Enterprise Search artifacts for years, I think we're probably fine.

Doesn't hurt to ask, but I wouldn't let it block you merging this. They can sometimes take a week or two to respond if they're busy.

@navarone-feekery navarone-feekery requested review from artem-shelkovnikov and a team May 16, 2024 08:10
@@ -0,0 +1,2459 @@
JRuby is Copyright (c) 2007-2018 The JRuby project, and is released
Copy link
Member

Choose a reason for hiding this comment

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

What about this and other manually added licences? We're forced to keep them?

Copy link
Collaborator Author

@navarone-feekery navarone-feekery May 21, 2024

Choose a reason for hiding this comment

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

I want them to remain because if they aren't there, every time someone wants to run make notice they will need to go and look for these files themselves and add it to this directory.

Copy link
Member

@artem-shelkovnikov artem-shelkovnikov left a comment

Choose a reason for hiding this comment

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

Looks good, asked one more question

@navarone-feekery navarone-feekery enabled auto-merge (squash) May 21, 2024 12:12
@navarone-feekery navarone-feekery merged commit 0e94a88 into main May 21, 2024
1 check passed
@navarone-feekery navarone-feekery deleted the navarone/add-noticefile-generator branch May 21, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants