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

fix license header normalizer #142

Merged
merged 1 commit into from
Oct 27, 2022

Conversation

xiaoyawei
Copy link
Contributor

Why do we need this fix?

As we are using license-eyes to check license header in our open-sourced projects, we notice that it can help detect
unexpected changes in the license header, except the copyright line. In my opinion, it's also necessary to protect the copyright line from unintentional changes.

How was this issue introduced?

#46 introduced this line processor to ignore copyright line, mostly for the purpose of dependency resolution. However, in #107 starts using Google's license check for this purpose, and this line processor only works to ignore copyright line when checking / fixing license header.

Reviewer

@kezhenxu94

@wu-sheng wu-sheng added the bug Something isn't working label Oct 27, 2022
@wu-sheng wu-sheng added this to the 0.5.0 milestone Oct 27, 2022
@kezhenxu94 kezhenxu94 merged commit 006a151 into apache:main Oct 27, 2022
@xiaoyawei xiaoyawei deleted the update_one_line_normalizer branch October 27, 2022 10:26
@tisonkun
Copy link
Member

This commit causes a previously passed CI to fail. You may re-check the diff: https://github.com/tisonkun/failpoints/actions/runs/3342840329/jobs/5535474781

@xiaoyawei
Copy link
Contributor Author

@tisonkun your CI failures look reasonable to me: its copyright line in license header does not match the specs in https://github.com/tisonkun/failpoints/blob/main/tools/ci/licenserc.yml

@kezhenxu94
Copy link
Member

kezhenxu94 commented Oct 28, 2022

I knew this was a breaking change, but it breaks something that was wrongly configured, and the previous successful CI should have been failed.

In your case, using spdx-id whose content has variables should be substituted, so you might need to provide copyright-owner, I know that would also fail because of a bug #143, the variables are not substituted previously.

@kezhenxu94
Copy link
Member

@tisonkun with #143 and the following patch to your repo, CI should pass

diff --git a/tools/ci/licenserc.yml b/tools/ci/licenserc.yml
index 7122e11..2c56a5d 100644
--- a/tools/ci/licenserc.yml
+++ b/tools/ci/licenserc.yml
@@ -15,6 +15,7 @@
 header:
   license:
     spdx-id: Apache-2.0
+    copyright-owner: tison <wander4096@gmail.com>
   paths:
     - '**/*.java'
     - '**/*.yml'

Let me know if that fix makes sense to you.

@tisonkun
Copy link
Member

@kezhenxu94 fair enough. Apply and fix the issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants