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

Enable configuring of categories in CRD generation #6457

Merged
merged 4 commits into from
Nov 27, 2024

Conversation

MichaelMorrisEst
Copy link
Contributor

Description

Adds a new annotation @Categories that enables users of the CRD generator to set categories in the generated CRD.

Closes #5795

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@MichaelMorrisEst MichaelMorrisEst marked this pull request as ready for review October 31, 2024 12:01
Copy link
Contributor

@baloo42 baloo42 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

Copy link
Collaborator

@metacosm metacosm left a comment

Choose a reason for hiding this comment

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

LGTM

@manusa manusa self-assigned this Nov 26, 2024
@@ -0,0 +1,36 @@
/*
* Copyright (C) 2024 Ericsson Software Technology
Copy link
Member

Choose a reason for hiding this comment

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

Hi @MichaelMorrisEst
The header/license files cannot be changed.
Is it OK if you fallback to the project template?
If not, I'm afraid we won't be able to merge this PR and discard its content.

#2045

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @manusa
I believe I have used the standard Apache 2.0 license verbatim and is identical to the header used in the other files (aside from the company name), so I am not sure what the issue is?

Copy link
Member

Choose a reason for hiding this comment

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

The company name. Style checks and license checks won't pass unless the header text is identical to the rest.

In case I need to make changes to the template or any of its variables I would need to discuss with Red Hat legal first.
In any case, at most the {owner} name could be changed to something like The Fabric8 Kubernetes Client authors (after their approval).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated as suggested to allow the PR to proceed, but I do not think this is in keeping with the norms for Apache 2.0 licensed software. Id suggest you should check with Red Hat legal what company name they expect in the case of contributions from companies other than Red Hat. I believe the norm here is the company name of the contributor.

Copy link
Member

Choose a reason for hiding this comment

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

As far as I've seen in any other community project, especially those owned by foundations, it's rarely the case that the company/companies is/are cited in the license header of a file.
Usually a file is contributed to by many individuals who (in a very healthy project) might work for multiple companies.
The file might end up with multiple copyright owner lines.
In our case, I'm unsure if this might have any additional repercussions, hence having to check with legal first for any changes to the licensing status quo.

The header is kind of useless nonetheless. Most projects are shifting to SPDX license expressions, and we should too.
This is just one of those low priority chores that need to be taken care of but haven't had the time to deal with yet. 😓

Signed-off-by: MichaelMorris <michael.morris@est.tech>
Signed-off-by: MichaelMorris <michael.morris@est.tech>
Signed-off-by: MichaelMorris <michael.morris@est.tech>
Signed-off-by: MichaelMorris <michael.morris@est.tech>
@manusa manusa added this to the 7.0.0 milestone Nov 27, 2024 — with automated-tasks
Copy link
Member

@manusa manusa left a comment

Choose a reason for hiding this comment

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

LGTM, thx! also for your patience and cooperation 🙌

@manusa manusa merged commit 51f8204 into fabric8io:main Nov 27, 2024
20 checks passed
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.

CRDGenerator: Allow to configure categories
4 participants