-
Notifications
You must be signed in to change notification settings - Fork 225
NatssChannel Dispatcher: prioritize finalizer removal #824
NatssChannel Dispatcher: prioritize finalizer removal #824
Conversation
Hi @nachtmaar. 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. Instructions 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. |
/assign @Abd4llA |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
/ok-to-test |
7748a34
to
c5df5ad
Compare
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
c5df5ad
to
2ac81c6
Compare
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only minor comments, good job!
/lgtm |
/test pull-knative-eventing-contrib-integration-tests |
This reverts commit e6d4ad4.
…ame"" This reverts commit fc5c4f9.
/lgtm |
@k15r: changing LGTM is restricted to collaborators In response to this:
Instructions 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. |
The following is the coverage report on the affected files.
|
/test pull-knative-eventing-contrib-integration-tests |
/lgtm |
@k15r: changing LGTM is restricted to collaborators In response to this:
Instructions 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. |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Abd4llA, k15r, nachtmaar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Proposed Changes
Remove natsschannel finalizer as early as possible
Add simple test to validate that finalizer on natsschannel gets removed even though natss channel is not ready
Add NatssDispatcher interface
removed unreferenced
toSubscriberStatus
functionadd typedef
SubscriptionChannelMapping
Bumped code coverage to 83 % ( I din't see that code coverage job is optional :/ )
Release Note
See also kyma-project/kyma#6792