Skip to content

Conversation

ldevulder
Copy link
Member

Add missing headers in Go files to make golangci-lint happy.

@ldevulder ldevulder requested a review from a team as a code owner March 5, 2025 09:55
Copy link
Contributor

@atanasdinov atanasdinov left a comment

Choose a reason for hiding this comment

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

Can we use 2025 only everywhere? 👀

@ldevulder
Copy link
Member Author

Can we use 2025 only everywhere?

The issue is that some code will be copy/paste from current Elemental code (see PR #2) and not sure if the linter will be happy. Will check.

@atanasdinov
Copy link
Contributor

You're right. I guess maybe we can actively adjust the code to point to 2025 instead? The linter will fail unless the new files are adjusted and I guess it makes sense to go for 2025 even if some of the code is completely copy-pasted. Thoughts?

@ldevulder
Copy link
Member Author

I checked in others projects in Rancher and some are using regex for this, will try to use this. I think it's better to keep the original date for older files.

@ldevulder
Copy link
Member Author

ldevulder commented Mar 5, 2025

Using {{year-range}} should be fine for almost all cases.

New files should be like this: Copyright © 2025 SUSE LLC and modified/copied files should be for example: Copyright © 2022-2025 SUSE LLC.

@davidcassany the internal {{year-range}} check uses the syntax year1-year2 and not the one we have in older Elemental code which is year1 - year2... So I can create a regex to match the old behavior or we can fix the copied code in the PR with a sed for example, WDYT is the best/easiest?

@ldevulder ldevulder force-pushed the add-missing-goheaders branch from fdf83ab to b9fecc1 Compare March 5, 2025 11:28
Signed-off-by: Loic Devulder <[email protected]>
@ldevulder ldevulder force-pushed the add-missing-goheaders branch from b9fecc1 to c1c6258 Compare March 5, 2025 12:40
@fdegir
Copy link
Member

fdegir commented Mar 5, 2025

I have two comments about this, not specific to this PR or the files in this repository but a general approach to copyright notices, license files and so on. Please note that I'm not sure if SUSE has a policy around this and if so, what I suggest below may not be valid.

  • There is a movement to drop years in the license headers in individual files to reduce the maintenance burden and avoid inconsistencies. [1][2][3]
  • I suggest using SPDX compliant license IDs in the headers to allow license information to be easily extracted by scripts or code scanning tools. [4]

[1] https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects
[2] https://daniel.haxx.se/blog/2023/01/08/copyright-without-years/
[3] https://hynek.me/til/copyright-years/
[5] https://spdx.dev/learn/handling-license-info/

@davidcassany
Copy link
Contributor

@davidcassany the internal {{year-range}} check uses the syntax year1-year2 and not the one we have in older Elemental code which is year1 - year2... So I can create a regex to match the old behavior or we can fix the copied code in the PR with a sed for example, WDYT is the best/easiest?

I'd simply fix copied code and adapt wherever necessary. In any case if we can avoid having the year there that would be even better. So I think we should try to figure out the actual requirements for the header first.

@ldevulder
Copy link
Member Author

  • There is a movement to drop years in the license headers in individual files to reduce the maintenance burden and avoid inconsistencies. [1][2][3]

Yes but legal team asked for this some times ago. See ou Slack channel for a more detailed answer.

  • I suggest using SPDX compliant license IDs in the headers to allow license information to be easily extracted by scripts or code scanning tools. [4]

I was thinking of this yes, but we followed the Apache license rules, but I think we can add it without any issue. Any comment from others on this? I can add it anyway.

@ldevulder
Copy link
Member Author

So I think we should try to figure out the actual requirements for the header first.

More detailed answer in the Slack channel, but it seems that we should keep it as-is.

@ldevulder ldevulder force-pushed the add-missing-goheaders branch from c1c6258 to d460008 Compare March 5, 2025 14:26
Signed-off-by: Loic Devulder <[email protected]>
@ldevulder ldevulder force-pushed the add-missing-goheaders branch 2 times, most recently from 3e10922 to 19f89fb Compare March 5, 2025 15:29
@ldevulder
Copy link
Member Author

The lint errors/warnings on the code itself will be fixed in a separate PR.

@atanasdinov atanasdinov requested a review from a team March 5, 2025 15:36
Copy link
Contributor

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

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

LGTM

@ldevulder ldevulder merged commit 9745a2e into SUSE:main Mar 5, 2025
0 of 2 checks passed
@ldevulder ldevulder deleted the add-missing-goheaders branch March 5, 2025 15:47
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.

4 participants