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

Replace Apache license headers with SPDX ID #5808

Merged

Conversation

thecaffeinedev
Copy link
Contributor

@thecaffeinedev thecaffeinedev commented Aug 6, 2024

Which problem is this PR solving?

Description of the changes

  • Added a new Python script update_license_headers.py in the ./scripts/ directory
  • Replaces old Apache headers with SPDX identifiers
  • Works on .go files
  • No changes to source files are included in this PR.

How was this change tested?

  • I gave it a spin on the cmd folder. Seemed to work fine - it found the right files and swapped out the headers. I checked a few of the updated files manually to make sure it did what it was supposed to.

Checklist

Signed-off-by: Prabhat <iprabhatdev@gmail.com>
@thecaffeinedev thecaffeinedev requested a review from a team as a code owner August 6, 2024 07:51
@rnd-debug
Copy link

datetime doesn't seem to be used, but is still imported.

Signed-off-by: Prabhat Kumar Sahu <iprabhatdev@gmail.com>
@thecaffeinedev
Copy link
Contributor Author

datetime doesn't seem to be used, but is still imported.

Thanks for the review. I was looking at this updateLicense.py for the reference. I have updated the code.

@yurishkuro
Copy link
Member

I merged main, please do a pull to get the merge commit before any future changes

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

I tried on one file and got this diff

 // Copyright (c) 2019 The Jaeger Authors.
 // Copyright (c) 2017 Uber Technologies, Inc.
 //
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+// SPDX-License-Identifier: Apache-2.0

I don't think we need the empty comment after Copyright lines, remove it too. This is what we want:

// Copyright (c) 2023 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package main

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.66%. Comparing base (eaacbf0) to head (cf31464).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5808      +/-   ##
==========================================
+ Coverage   96.65%   96.66%   +0.01%     
==========================================
  Files         342      342              
  Lines       16525    16525              
==========================================
+ Hits        15972    15974       +2     
+ Misses        363      362       -1     
+ Partials      190      189       -1     
Flag Coverage Δ
badger_v1 8.05% <ø> (ø)
badger_v2 1.81% <ø> (ø)
cassandra-3.x-v1 16.61% <ø> (ø)
cassandra-3.x-v2 1.74% <ø> (ø)
cassandra-4.x-v1 16.61% <ø> (ø)
cassandra-4.x-v2 1.74% <ø> (ø)
elasticsearch-6.x-v1 18.77% <ø> (ø)
elasticsearch-7.x-v1 18.84% <ø> (ø)
elasticsearch-8.x-v1 19.03% <ø> (ø)
elasticsearch-8.x-v2 1.81% <ø> (ø)
grpc_v1 9.51% <ø> (ø)
grpc_v2 7.14% <ø> (ø)
kafka-v1 9.74% <ø> (ø)
kafka-v2 1.81% <ø> (ø)
memory_v2 1.81% <ø> (+0.01%) ⬆️
opensearch-1.x-v1 18.88% <ø> (ø)
opensearch-2.x-v1 18.89% <ø> (+0.01%) ⬆️
opensearch-2.x-v2 1.81% <ø> (+0.01%) ⬆️
unittests 95.09% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

thecaffeinedev and others added 4 commits August 8, 2024 10:19
Signed-off-by: Prabhat Kumar Sahu <iprabhatdev@gmail.com>
Signed-off-by: Prabhat Kumar Sahu <iprabhatdev@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro added changelog:bugfix-or-minor-feature changelog:skip Trivial change that does not require an entry in CHANGELOG labels Aug 8, 2024
@yurishkuro
Copy link
Member

There is a bug: when I run this script on cmd/agent/app/agent.go it incorrectly deletes one of the Copyright lines. It must only replace the copyright header, and ideally it should actually fully match on that header, to ensure we're not replacing some other wording (which may be non-Apache 2.0)

@thecaffeinedev
Copy link
Contributor Author

There is a bug: when I run this script on cmd/agent/app/agent.go it incorrectly deletes one of the Copyright lines. It must only replace the copyright header, and ideally it should actually fully match on that header, to ensure we're not replacing some other wording (which may be non-Apache 2.0)

Thank you for pointing out the bug. I have updated the script to address the issue.

I have tested the script on cmd/agent/app/agent.go and other files, and it now correctly preserves all copyright lines while adding the SPDX identifier.

scripts/replace_license_headers.py Dismissed Show dismissed Hide dismissed
Signed-off-by: Yuri Shkuro <github@ysh.us>
This reverts commit 80c6f1c.

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro enabled auto-merge (squash) August 14, 2024 04:17
@yurishkuro yurishkuro changed the title Add script to replace Apache headers with SPDX Replace Apache license headers with SPDX ID Aug 14, 2024
@yurishkuro yurishkuro merged commit cc83c76 into jaegertracing:main Aug 14, 2024
43 checks passed
@yurishkuro
Copy link
Member

Thanks!

JaredTan95 pushed a commit to JaredTan95/jaeger that referenced this pull request Aug 28, 2024
## Which problem is this PR solving?
- Resolves jaegertracing#5807

## Description of the changes
- Added a new Python script `update_license_headers.py` in the
`./scripts/` directory
- Replaces old Apache headers with SPDX identifiers
- Works on .go files
- No changes to source files are included in this PR.

## How was this change tested?
- I gave it a spin on the `cmd` folder. Seemed to work fine - it found
the right files and swapped out the headers. I checked a few of the
updated files manually to make sure it did what it was supposed to.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Prabhat <iprabhatdev@gmail.com>
Signed-off-by: Prabhat Kumar Sahu <iprabhatdev@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Jared Tan <jian.tan@daocloud.io>
@yurishkuro yurishkuro removed the changelog:skip Trivial change that does not require an entry in CHANGELOG label Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write a script to replace long Apache license headers with a short SPDX reference.
3 participants