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

Deprecate PrinterColumn annotation to move it to the crd-generator module #5342

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

andreaTP
Copy link
Member

Description

Follow up from #5339

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

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 (needs running spotless to pass style checks)

@andreaTP
Copy link
Member Author

FYI @bachmanity1

@manusa manusa added this to the 6.8.0 milestone Jul 20, 2023
@andreaTP andreaTP force-pushed the deprecate-printer-col branch from 352e0f5 to 8efaf0a Compare July 20, 2023 10:35
@andreaTP
Copy link
Member Author

(needs running spotless to pass style checks)

fixed and squashed.

@@ -42,7 +42,7 @@ public enum ExcludedTopic {
@PrinterColumn(name = "jokeCategory", priority = 1)
@JsonPropertyDescription("category-description")
private Category category = Category.Any;
@PrinterColumn(name = "excludedTopics")
@io.fabric8.kubernetes.model.annotation.PrinterColumn(name = "excludedTopics")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're still using deprecated PrinterColumn here.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. I think imports in the io.fabric8.crd.generator.zookeeper.v1.ZookeeperStatus also must be updated because it still uses deprecated version of PrinterColumn.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's intentional, we want to test that both annotations will work.

Copy link
Contributor

Choose a reason for hiding this comment

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

then when do you plan to replace old annotations?

Copy link
Member Author

Choose a reason for hiding this comment

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

With the release of the next Minor, theoretically 6.9 at this point.

@andreaTP andreaTP force-pushed the deprecate-printer-col branch from 8efaf0a to fd12d6f Compare July 20, 2023 11:58
@manusa manusa requested a review from bachmanity1 July 20, 2023 12:07
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@manusa manusa merged commit 4d0afa2 into fabric8io:main Jul 20, 2023
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