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

lint: run goheader enforcing standardized Go header template #21821

Merged
merged 4 commits into from
Oct 21, 2022

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented Oct 20, 2022

Since we didn't have a linter to enforce a particular Go file header format, this patch adds one with a relatively simple structure.

goheader can only match on a single block of // statements, so anything that cannot be encoded in the template needed to be moved to a separate block.

For Hubble packages, 'Authors of Hubble' was kept as the primary tagline. Any other copyright notices were moved to subsequent blocks.

Fixes: #20769 (comment)

Fix up and lint SPDX headers in all Go files

@ti-mo ti-mo requested review from a team as code owners October 20, 2022 14:02
@ti-mo ti-mo requested a review from a team October 20, 2022 14:02
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 20, 2022
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

😲

@ti-mo ti-mo added the release-note/misc This PR makes changes that have no direct user impact. label Oct 20, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 20, 2022
@ti-mo ti-mo requested a review from a team as a code owner October 20, 2022 14:22
.golangci.yaml Show resolved Hide resolved
pkg/bpf/binary/binary.go Outdated Show resolved Hide resolved
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Based on the discussions on my prior feedback, I don't think any changes to the content are necessary. It'd be nice to update the commit messages to highlight the background to the changes but otherwise this LGTM. Thanks for this, it's caught a bunch of missing licenses in source files 🎉

ca451eb removed the use of SHT_REL(A), making this unnecessary.

Signed-off-by: Timo Beckers <timo@isovalent.com>
These files were mostly taken from the Go stdlib. It seems like the Go project
has also moved to a lean file header pointing to a single LICENSE file in the
project. Retain only the relevant Go Authors copyright notice and point to the
original files.

Signed-off-by: Timo Beckers <timo@isovalent.com>
Signed-off-by: Timo Beckers <timo@isovalent.com>
Since we didn't have a linter to enforce a particular Go file header format,
this patch adds one with a relatively simple structure.

goheader can only match on a single block of // statements, so anything that
cannot be encoded in the template needed to be moved to a separate block.

For Hubble packages, 'Authors of Hubble' was kept as the primary tagline.
Any other copyright notices were moved to subsequent blocks.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 21, 2022
@ti-mo
Copy link
Contributor Author

ti-mo commented Oct 21, 2022

Hit some unrelated unit test flake in Travis, marking ready.

@joestringer joestringer merged commit 65420ba into cilium:master Oct 21, 2022
@ti-mo ti-mo deleted the tb/goheader-spdx branch October 24, 2022 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants