Bugfix: Rename CR kind to KnativeEventing #48
Conversation
|
Hi @cardil. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test |
|
/hold let's not merge YET |
cardil
changed the title
|
We should merge this after we cut for 0.11 - right ? Currently we have no such branch: |
Exactly. This should be for |
Plus a lint fix for README.md
|
/lgtm When the 0.11 cuts, we can let the hold off |
| plural: knativeeventings | ||
| singular: knativeeventing | ||
| shortNames: | ||
| - ke |
There was a problem hiding this comment.
I am now really leaning on to removing this short name. For example, if we have older version of knative eventing operator installed, this short name is occupied by the CRD eventings.operator.knative.dev. After we applying the new CRD knativeeventings.operator.knative.dev also with this same short name, the terrible thing will happen: none of the old or the new CR can be found by the operator controller. If we remove this short name or change it to something else, this issue is gone. @cardil @n3wscott
There was a problem hiding this comment.
That's a valid concern for me. I'm okay with removing it.
|
@cardil so, we have https://github.com/knative/eventing-operator/tree/v0.11.0 as the release for 0.11 was cut. wanna move forward here, cancel the HOLD ? |
There's a problem, that if we have older version of knative eventing operator installed, this short name is occupied by the CRD eventings.operator.knative.dev. After we applying the new CRD knativeeventings.operator.knative.dev also with this same short name, the terrible thing will happen: none of the old or the new CR can be found by the operator controller. Typically you won't deal with those resources on a daily basis, occupying a 2-letter slot seems to be intrusive and not really justified. Taking above to account, removing that short name seams most reasonable.
|
@matzew Please cancel the hold. Also, waiting for lgtm after additional remove of short name. |
|
/hold cancel |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cardil, matzew, n3wscott The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
6 participants
This PR renames eventing CR to be named
KnativeEventingand be aligned with knative serving.This is inline with Operation WG decision made on meeting of Dec 10th and should address #18