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

[camel-kamelets] Issue-1330: Deprecating parameters #4097

Merged
merged 3 commits into from
Mar 6, 2023

Conversation

manstis
Copy link

@manstis manstis commented Mar 3, 2023

apache/camel-kamelets#1330

I added the CRD field as instructed and similar to the JSONSchemaProp structure.

I checked generating the documentation with deprecated property and it is already handled.

Just the validation fails as the CRD schema lacks the property.

There is a generated file that looks as though it should be re-generated (since it handles the JSONSchemaProp structure) however I could not see how to regenerate it. The code comments in other files suggest a make step for generate-deepcopy but that appears to be long gone following refactoring of the repository over the years.

Advice is welcome.

Release Note

NONE

Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

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

You need to run make generate and add the generated code changes as well.

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

please generate the crd files through make generate

@manstis
Copy link
Author

manstis commented Mar 3, 2023

Thanks @squakez @oscerd.

IDK the CRDs were generated from the sources.

Done.

Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

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

You need to align the controller-gen to the version we're using or alternatively, revert those changes that have nothing to deal with the deprecate stuff.

@@ -19,7 +19,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.6.1
controller-gen.kubebuilder.io/version: v0.9.2
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not look good. I think you have some controller-gen version in your local environment different than the one we are using.

Copy link
Author

@manstis manstis Mar 3, 2023

Choose a reason for hiding this comment

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

I did wonder.. fixed (I regenerated with controller-gen@v0.6.1)

@oscerd
Copy link
Contributor

oscerd commented Mar 3, 2023

I dont know if it makes sense to bring this down on 1.12.x too, it should be harmless but it's a breaking change.

@manstis
Copy link
Author

manstis commented Mar 3, 2023

I dont know if it makes sense to bring this down on 1.12.x too, it should be harmless but it's a breaking change.

Let me know.. it's your call.

@squakez
Copy link
Contributor

squakez commented Mar 3, 2023

I dont know if it makes sense to bring this down on 1.12.x too, it should be harmless but it's a breaking change.

I don't think it's a breaking change as we introduce a parameter which could be optional. Whatever works now in 1.12 will work after the change. Feel free to backport.

@manstis
Copy link
Author

manstis commented Mar 3, 2023

@squakez Do you mean release-1.12.x?

I couldn't see a 1.12.x otherwise.

@oscerd
Copy link
Contributor

oscerd commented Mar 3, 2023

yes, it's release-1.12.x

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.

None yet

3 participants