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

Integrate SkyWalking-eyes to check license headers in CI #26956

Closed
fgksgf opened this issue Aug 6, 2021 · 10 comments · Fixed by #27198
Closed

Integrate SkyWalking-eyes to check license headers in CI #26956

fgksgf opened this issue Aug 6, 2021 · 10 comments · Fixed by #27198
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@fgksgf
Copy link
Member

fgksgf commented Aug 6, 2021

Enhancement

Integrate SkyWalking-eyes to check license headers in CI.

For some new contributors, they may do not know the license header is required, we can use the GitHub Action and check license headers in CI automatically.

@fgksgf fgksgf added the type/enhancement The issue or PR belongs to an enhancement. label Aug 6, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Aug 6, 2021

Hi @fgksgf , Thanks for creating this issue!

I think it is a good point we enable license checker in CI. In my opinion, you can directly create a PR and let's see how this action works on our repo. Then if it is good to go, we push it to merge.

@fgksgf
Copy link
Member Author

fgksgf commented Aug 6, 2021

Hi @fgksgf , Thanks for creating this issue!

I think it is a good point we enable license checker in CI. In my opinion, you can directly create a PR and let's see how this action works on our repo. Then if it is good to go, we push it to merge.

No problem, I will do this :)

@fgksgf
Copy link
Member Author

fgksgf commented Aug 13, 2021

I locally checked the project with SkyWalking-eyes, and found that almost all license headers have one less sentence compared to the standard one.

@kezhenxu94
Copy link

kezhenxu94 commented Aug 13, 2021

I locally checked the project with SkyWalking-eyes, and found that almost all license headers have one less sentence compared to the standard one.

Well, the missing line is "WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.", I think this sentence is critical and a modified Apache license header is not called Apache license actually. It's better to add it back.

@fgksgf
Copy link
Member Author

fgksgf commented Aug 13, 2021

I locally checked the project with SkyWalking-eyes, and found that almost all license headers have one less sentence compared to the standard one.

Well, the missing line is "WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.", I think this sentence is critical and a modified Apache license header is not called Apache license actually. It's better to add it back.

I'm working on fixing them.

@dragonly
Copy link
Contributor

dragonly commented Aug 18, 2021

Congrats! You just fixed an error from the Genesis Commit!!!!!!

https://github.com/pingcap/tidb/blob/0d6f270068e8ff2aedc1c314e907771b6a508ebd/session.go

@fgksgf
Copy link
Member Author

fgksgf commented Aug 18, 2021

@tisonkun We may need create an issue to track the ignored files in .github/.licenserc.yaml.

@tisonkun
Copy link
Contributor

@tisonkun We may need create an issue to track the ignored files in .github/.licenserc.yaml.

Yes. Would you like to create one for them?

@tisonkun
Copy link
Contributor

I'm creating the issue...

@tisonkun
Copy link
Contributor

as #27356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants