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.

Sign up for GitHub

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

Jump to bottom

#2779 CrdGenerator validate @JsonPropertyOrder #9831

Merged
merged 11 commits into from
Mar 14, 2024
Merged

#2779 CrdGenerator validate @JsonPropertyOrder #9831

merged 11 commits into from
Mar 14, 2024

Conversation

steffen-karlsson
Copy link

@steffen-karlsson steffen-karlsson commented Mar 13, 2024 •
edited
Loading

Type of change

Description

Validating existence of @JsonPropertyOrder and that all relevant fields in class are present in the @JsonPropertyOrder

Checklist

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

@steffen-karlsson steffen-karlsson force-pushed the 2779-validating-crdgenerator branch 2 times, most recently from ae8b9df to 34024bf Compare March 14, 2024 07:12
@steffen-karlsson steffen-karlsson marked this pull request as ready for review March 14, 2024 07:13
@steffen-karlsson steffen-karlsson force-pushed the 2779-validating-crdgenerator branch 2 times, most recently from 069620b to 9cbc7e1 Compare March 14, 2024 07:54
Steffen Karlsson added 11 commits March 14, 2024 10:03
Enforcing @JsonPropertyOrder on all implementation classes
d60ac9b 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Adding dummy @JsonPropertyOrder on all classes to prevent error fro…
dd28e0d 
…m happening while compiling

Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Check on property existence in the JsonPropertyOrder
ba08a30 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Adding all properties to the JsonPropertyOrder
99eac5a 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Javadoc not working without default constructor docs
dffc662 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Adding new CRDS with property changes from updated JsonPropertyOrder
82bf09f 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Do not do class annotation validation for java.lang.* classes
16f81a8 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Fixing CRD Generator tests
2fcd3df 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Fix codestyle check
0b0956a 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Fix doc test with new property order
55b3a38 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
Fixed API CRD tests due to JsonPropertyOrder changes
e3f98ee 
Signed-off-by: Steffen Karlsson <steffen.karlsson@maersk.com>
@steffen-karlsson steffen-karlsson force-pushed the 2779-validating-crdgenerator branch from 0502e5c to e3f98ee Compare March 14, 2024 09:04
@scholzj scholzj linked an issue Mar 14, 2024 that may be closed by this pull request
[Enhancement] CrdGenerator validate @JsonPropertyOrder #2779
Closed
@scholzj scholzj added this to the 0.41.0 milestone Mar 14, 2024
@scholzj scholzj requested a review from ppatierno March 14, 2024 09:17
scholzj
scholzj reviewed Mar 14, 2024
View reviewed changes
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I think this looks good - I didn't realized we were missing it on so many classes. I will run the regression tests on this as well.

@steffen-karlsson
Copy link
Author

Thanks for the PR. I think this looks good - I didn't realized we were missing it on so many classes. I will run the regression tests on this as well.

Was also surprised when I started adjusting the classes :D

@scholzj
Copy link
Member

scholzj commented Mar 14, 2024

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

scholzj
scholzj approved these changes Mar 14, 2024
View reviewed changes
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR.

ppatierno
ppatierno approved these changes Mar 14, 2024
View reviewed changes
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@scholzj scholzj merged commit 3f9fe6f into strimzi:main Mar 14, 2024
21 checks passed
@scholzj scholzj mentioned this pull request Nov 29, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scholzj scholzj scholzj approved these changes

@ppatierno ppatierno ppatierno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.41.0
Development

Successfully merging this pull request may close these issues.

[Enhancement] CrdGenerator validate @JsonPropertyOrder
3 participants
@steffen-karlsson @scholzj @ppatierno