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 headers #659

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Fix license headers #659

merged 4 commits into from
Feb 26, 2024

Conversation

radeksimko
Copy link
Member

This aims to fix a number of problems:

  • one static file had a missing header, as surfaced in [COMPLIANCE] Add Copyright and License Headers #637
  • some generated files had license headers added by the copywrite tool without reflecting the fact they are generated (and that they should get generated with the headers already)
  • go generate ./... implies a dependency on stringer, which is a Go library that can be tracked as such to provide reproducible results.
    • There are actually more dependencies which we cannot track like this but I am just addressing one problem at a time here.
      • Installation of Ragel on Linux/Windows/macOS would be an achievement of its own.
      • That one Ruby script could probably be rewritten into Go rather than us having to depend on Ruby

Closes #637


Next step, which I'm expecting to be a separate PR, would be to add some ways of checking for license headers both in the CI and locally, perhaps similar to what we do in hashicorp/terraform.

In theory we could use the latest (v0.18.0) version of the x/tools module but that would also bring upgrade of x/sys (transitive dependency of go-cty) from current v0.5.0 and full understanding of implications of that upgrade.
@@ -263,7 +262,7 @@ var _hcltok_trans_keys []byte = []byte{
233, 234, 237, 239, 240, 243, 48, 57,
65, 90, 97, 122, 196, 218, 229, 236,
10, 170, 181, 183, 186, 128, 150, 152,
182, 184, 255, 192, 255, 128, 255, 173,
182, 184, 255, 192, 255, 0, 127, 173,
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure about the meaning of this byte slice but my understanding is that it will change even with a single character change in the corresponding Ragel file it is generated from.

Since two lines of license headers are being added there, that would explain this.

@@ -4,6 +4,24 @@ package json

import "strconv"

func _() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This was added back in March 2019 to stringer in https://go-review.googlesource.com/c/tools/+/163637 which implies we have just not run stringer ever since it released the first stable version (v0.1.0 was released in January 2021).

I avoided pinning to some unstable old version just for that purpose and picked the latest compatible stable version.

@radeksimko radeksimko requested a review from a team February 16, 2024 12:49
@radeksimko radeksimko marked this pull request as ready for review February 16, 2024 12:49
@radeksimko radeksimko merged commit 76bf60b into main Feb 26, 2024
7 checks passed
@radeksimko radeksimko deleted the fix-license-headers branch February 26, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants