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 SPDX header - batch #2 #45252

Merged
merged 3 commits into from
May 13, 2024
Merged

Add SPDX header - batch #2 #45252

merged 3 commits into from
May 13, 2024

Conversation

AndyScherzinger
Copy link
Member

@AndyScherzinger AndyScherzinger commented May 10, 2024

Summary

Add further SPDX header or reformat existing headers for the converter to work


Before

  • files with copyright information: 8292 / 10921
  • files with license information: 4242 / 10921

After

  • files with copyright information: 8687 / 10921
  • files with license information: 5105 / 10921

TODO

  • Migrate further js resources

Checklist

@AndyScherzinger AndyScherzinger added this to the Nextcloud 30 milestone May 10, 2024
@AndyScherzinger AndyScherzinger changed the title Add SPDX header - batch 2 Add SPDX header - batch #2 May 10, 2024
@AndyScherzinger

This comment was marked as outdated.

@AndyScherzinger
Copy link
Member Author

@susnux, can you please re-review. I triggered the compile and before that pushed all headers for core/js 🎉

@AndyScherzinger AndyScherzinger added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 10, 2024
@AndyScherzinger
Copy link
Member Author

Any hints as to why the changed/now SPDX header won't get copied over? see i.e. 95f1c09#diff-21c6a91f3d35ef70889606c918fa3ee8d5dbd41e0983ccba0514c49263065ba5

@susnux
Copy link
Contributor

susnux commented May 10, 2024

@AndyScherzinger yes because by default the regex to detect licenses does not include the spdx identifiers.
But instead of adjusting that I would prefer to properly extract the licenses for a bill of materials, what do you think?
Example: #45259

core/css/inputs.scss Outdated Show resolved Hide resolved
@AndyScherzinger AndyScherzinger force-pushed the chore/noid/spdx-batch2 branch 4 times, most recently from accc95b to 8a06a01 Compare May 11, 2024 14:43
@AndyScherzinger
Copy link
Member Author

/compile

@susnux
Copy link
Contributor

susnux commented May 13, 2024

@AndyScherzinger despite of #45259 I pushed a commit that fixes extracting the comments (added @copyright and @spdx as pattern for comments to extract).

@AndyScherzinger
Copy link
Member Author

@susnux sounds good, however the SPDX headers don't start with an @, not sure of that is relevant. However some license files seem to be gone completely now, not sure if that is also related to the change you did. best case we "just" have .license files for all files in the dist folder

@AndyScherzinger AndyScherzinger force-pushed the chore/noid/spdx-batch2 branch 3 times, most recently from 8e72f8a to 3296936 Compare May 13, 2024 10:14
@susnux
Copy link
Contributor

susnux commented May 13, 2024

however the SPDX headers don't start with an @, not sure of that is relevant.

Yes, fixed that.

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Squash?

@susnux
Copy link
Contributor

susnux commented May 13, 2024

image

AndyScherzinger and others added 3 commits May 13, 2024 17:41
Signed-off-by: Andy Scherzinger <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented May 13, 2024

Commits combined so they still "make sense" and are specific to their actual character of change

@AndyScherzinger AndyScherzinger merged commit e3f341f into master May 13, 2024
155 checks passed
@AndyScherzinger AndyScherzinger deleted the chore/noid/spdx-batch2 branch May 13, 2024 16:37
@blizzz blizzz mentioned this pull request Jul 24, 2024
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.

5 participants