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

Minor fix for missing allowed key type #2247

Merged
merged 2 commits into from
Jun 7, 2022

Conversation

aavarghese
Copy link
Contributor

@aavarghese aavarghese commented Jun 2, 2022

Signed-off-by: aavarghese avarghese@us.ibm.com

Related to #2244

Fixes a problem in #2219

Part of #1537

@knative-prow knative-prow bot added area/control-plane size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 2, 2022
@aavarghese
Copy link
Contributor Author

aavarghese commented Jun 2, 2022

/cc @matzew @aslom @pierDipi

@knative-prow knative-prow bot requested review from aslom and matzew June 2, 2022 14:24
@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #2247 (1670f5c) into main (0a74971) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #2247      +/-   ##
============================================
+ Coverage     65.61%   65.62%   +0.01%     
- Complexity      697      701       +4     
============================================
  Files           140      140              
  Lines          9158     9164       +6     
  Branches        208      208              
============================================
+ Hits           6009     6014       +5     
- Misses         2748     2752       +4     
+ Partials        401      398       -3     
Flag Coverage Δ
java-unittests 82.27% <ø> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...nternals/kafka/eventing/v1alpha1/consumer_types.go 79.41% <ø> (ø)
...als/kafka/eventing/v1alpha1/consumer_validation.go 77.08% <0.00%> (-5.14%) ⬇️
...a/broker/dispatcher/impl/RecordDispatcherImpl.java 88.39% <0.00%> (+0.55%) ⬆️
...patcher/impl/consumer/OrderedConsumerVerticle.java 75.00% <0.00%> (+2.17%) ⬆️
...ispatcher/impl/http/WebClientCloudEventSender.java 62.79% <0.00%> (+2.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a74971...1670f5c. Read the comment docs.

@aslom
Copy link
Member

aslom commented Jun 2, 2022

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 2, 2022
Copy link
Contributor

@matzew matzew left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 3, 2022
@matzew
Copy link
Contributor

matzew commented Jun 3, 2022

@knative-prow knative-prow bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 3, 2022
Signed-off-by: aavarghese <avarghese@us.ibm.com>
Signed-off-by: aavarghese <avarghese@us.ibm.com>
@aavarghese
Copy link
Contributor Author

/test unit-tests_eventing-kafka-broker_main

@aavarghese
Copy link
Contributor Author

actually, can't we just use https://github.com/knative-sandbox/eventing-kafka/blob/main/pkg/apis/sources/v1beta1/kafka_types.go#L103

@matzew yeah, we should. Made that change in a new commit. Thanks!

@aavarghese aavarghese requested a review from matzew June 7, 2022 14:28
@aavarghese
Copy link
Contributor Author

/test integration-tests_eventing-kafka-broker_main

Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

Thank you!

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2022
@knative-prow
Copy link

knative-prow bot commented Jun 7, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aavarghese, matzew, pierDipi

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@aavarghese
Copy link
Contributor Author

/unhold

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 7, 2022
@knative-prow knative-prow bot merged commit b8fd4f7 into knative-extensions:main Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants