-
Notifications
You must be signed in to change notification settings - Fork 775
Introduce REUSE compliance #1613
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1613 +/- ##
=========================================
Coverage 97.77% 97.77%
Complexity 1007 1007
=========================================
Files 212 212
Lines 2339 2339
=========================================
Hits 2287 2287
Misses 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2f909ea to
46bfb71
Compare
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.
Pull request overview
This pull request introduces REUSE compliance to standardize licensing and copyright attribution across the project. The change replaces personal copyright claims with "The Respect Project Contributors" and implements SPDX headers throughout the codebase for better license tracking and attribution.
Changes:
- Removed the docheader tool and added REUSE compliance tooling
- Updated copyright attribution from individual name to "Respect Project Contributors"
- Added SPDX license headers to all source files and documentation
Reviewed changes
Copilot reviewed 298 out of 1102 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| composer.json | Removed malukenho/docheader dependency and related scripts |
| LICENSE | Updated copyright holder to "Respect Project Contributors" |
| REUSE.toml | Added REUSE configuration for license compliance |
| LICENSES/*.txt | Added license text files for MIT, MPL-2.0, and CC-BY-4.0 |
| .docheader | Removed obsolete docheader configuration |
| .github/workflows/*.yml | Added SPDX headers to workflow files |
| docs/**/*.md | Added SPDX headers in HTML comment format |
| data/postal-code.php | Updated copyright attribution and added new postal code patterns |
| data/domain/**/*.php | Updated copyright attribution from generic to Mozilla Foundation |
| aliases.php | Updated to SPDX format with contributor attribution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
henriquemoody
left a comment
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 think we don't need to add this to all files, just the files that are really ours, that are part of the library. That would not include the documentation files, though.
src-dev/Markdown/SpdxHeader.php
Outdated
| use function array_shift; | ||
| use function preg_match; | ||
|
|
||
| trait SpdxHeader |
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 don't think this belongs in this namespace
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.
Where would you put it?
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.
Since we're using that to handle the Content, I think this should be part of the content. You would then not even have a prependSpdxToContent, you would just do something like:
- $content = new Content();
+ $content = $file->content->extractSpdxHeader();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 was not able to make this work in a single method.
There are changes to $lines that occour outside of the Content class. For example, in ValidatorHeaderLinter:
while (($line = array_shift($lines)) !== false) {
if (preg_match('/^(# .+|- .+|)$/', $line) === 0) {
array_unshift($lines, $line);
break;
}
}I could not make it a single method because of it.
So, I definitely can move the methods to Content, but I was not able to make it a single clean API without further extensive refactorings.
Perhaps I didn't understood your idea or I'm missing something. If that's the case, can you clarify it?
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.
Yeah, I'm not a big fan of those changes outside the Content, but I decided to be less picky with it since we don't ship this code. You can merge as is. I was trying to make this a bit better, but I don't know how much better it is since we're making changes outside the Content as you pointed out.
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.
Nice, thanks for the diff! I decided to apply it because it leads to less lines of code.
|
Resolved the comments about the Markdown files. I had a conversation with @alganet, and he made me realize that it's as important to add that to the documentation files as to the code. |
8bab3f5 to
de969c8
Compare
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.
Pull request overview
Copilot reviewed 298 out of 1090 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This commit introduces REUSE compliance by annotating all files with SPDX information and placing the reused licences in the LICENSES folder. We additionally removed the docheader tool which is made obsolete by this change. The main LICENSE and copyright text of the project is now not under my personal name anymore, and it belongs to "The Respect Project Contributors" instead. This change restores author names to several files, giving the appropriate attribution for contributions.
de969c8 to
456ec30
Compare
This commit introduces REUSE compliance by annotating all files with SPDX information and placing the reused licences in the LICENSES folder.
We additionally removed the docheader tool which is made obsolete by this change.
The main LICENSE and copyright text of the project is now not under my personal name anymore, and it belongs to "The Respect Project Contributors" instead.
This change restores author names to several files, giving the appropriate attribution for contributions.