Add helper methods from in-memory provisioner to utils by neosab · Pull Request #560 · knative/eventing

neosab · 2018-10-24T21:09:23Z

Moving some methods from in-memory provisioner to a new channel_util & provisioner_util. These methods will be useful for other channel provisioners like kafka that I am working on.

neosab · 2018-10-24T21:17:39Z

/cc @adamharwayne

adamharwayne

Looks good, but now it needs unit tests. 😝

neosab · 2018-10-24T22:37:59Z

Looks good, but now it needs unit tests. 😝

Nothing comes for free 😞

adamharwayne · 2018-10-29T19:11:04Z

Please merge and I'll take another look.

neosab · 2018-10-29T19:14:31Z

@adamharwayne on it :)

neosab · 2018-10-29T21:33:07Z

/assign grantr

adamharwayne

/lgtm

adamharwayne · 2018-10-30T16:14:32Z

+	eventingv1alpha1.AddToScheme(scheme.Scheme)
+}
+
+func TestCreateK8sService(t *testing.T) {


My personal preference is table tests, but I'm not sure if that is something the repo has standardized on.

+1 on table tests for this situation where it is the same test body, but I would accept a followup for that refactor.

Sure, I will do. Opened up #572 to track it

adamharwayne · 2018-10-30T16:16:12Z

+		Spec: eventingv1alpha1.ChannelSpec{
+			Provisioner: &corev1.ObjectReference{
+				Name:       clusterChannelProvisionerName,
+				Kind:       "ClusterProvisioner",


ClusterChannelProvisioner

Thanks, I will address in the follow-up.

adamharwayne · 2018-10-30T16:18:31Z

+	return clusterChannelProvisioner
+}
+
+func ClusterProvisonerType() metav1.TypeMeta {


ClusterChannelProvisionerType()

n3wscott · 2018-10-31T14:33:37Z

/lgtm

renaming

googlebot · 2018-11-07T08:35:00Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that 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 here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

knative-metrics-robot · 2018-11-07T08:38:50Z

The following is the coverage report on pkg/.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/controller/eventing/inmemory/channel/reconcile.go	98.6%	97.9%	-0.7
pkg/provisioners/channel_util.go	Do not exist	85.1%
pkg/provisioners/provisioner_util.go	Do not exist	77.3%

matzew · 2018-11-07T18:36:35Z

@evankanderson can you set the CLA bot here too ?

matzew · 2018-11-07T18:37:11Z

would be good to land this too, @evankanderson

vaikas · 2018-11-07T20:28:27Z

/lgtm
/approve

knative-prow-robot · 2018-11-07T20:28:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neosab, vaikas-google

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [vaikas-google]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

neosab · 2018-11-07T21:28:36Z

I am 💯 happy to co-author this with @matzew

matzew · 2018-11-07T21:34:36Z

I am hppy w/ the changes too

/lgtm

googlebot · 2018-11-07T21:35:07Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

…ConfigMap (knative#560)

Add channel utils

ec5f434

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Oct 24, 2018

knative-prow-robot requested review from scothis and vaikas October 24, 2018 21:09

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 24, 2018

knative-prow-robot requested a review from adamharwayne October 24, 2018 21:17

adamharwayne reviewed Oct 24, 2018

View reviewed changes

Comment thread pkg/provisioners/channel_util.go Outdated

Comment thread pkg/provisioners/channel_util.go Outdated

neosab changed the title ~~Add helper methods from in-memory provisioner to channel_util~~ WIP: Add helper methods from in-memory provisioner to channel_util Oct 24, 2018

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 24, 2018

Add Provisioner util and fix comments

17f26d3

knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 24, 2018

neosab changed the title ~~WIP: Add helper methods from in-memory provisioner to channel_util~~ WIP: Add helper methods from in-memory provisioner to utils Oct 24, 2018

neosab added 3 commits October 25, 2018 15:28

Add unit tests

896cc3a

Merge remote-tracking branch 'upstream/master' into channel_utils

72a0817

Fix logging

e41e09a

neosab changed the title ~~WIP: Add helper methods from in-memory provisioner to utils~~ Add helper methods from in-memory provisioner to utils Oct 25, 2018

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 25, 2018

neosab added 2 commits October 29, 2018 12:15

Merge remote-tracking branch 'upstream/master' into channel_utils

8afafbf

Fix after pr 562

e28d777

knative-prow-robot assigned grantr Oct 29, 2018

adamharwayne reviewed Oct 30, 2018

View reviewed changes

knative-prow-robot assigned adamharwayne Oct 30, 2018

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2018

knative-prow-robot assigned n3wscott Oct 31, 2018

neosab mentioned this pull request Oct 31, 2018

Add control plane services for Kafka Channel Provisioner #573

Merged

matzew and others added 2 commits November 6, 2018 04:00

renaming

c969f92

Merge remote-tracking branch 'upstream/master' into channel_utils

7e72d7a

neosab unassigned adamharwayne Nov 7, 2018

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2018

Merge pull request #3 from matzew/channel_util_renames

66575c5

renaming

googlebot added cla: no Indicates the PR's author has not signed the CLA. and removed cla: yes Indicates the PR's author has signed the CLA. labels Nov 7, 2018

Fix import ordering and test

6c1571c

knative-prow-robot assigned vaikas Nov 7, 2018

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2018

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 7, 2018

knative-prow-robot assigned matzew Nov 7, 2018

vaikas added cla: yes Indicates the PR's author has signed the CLA. and removed cla: no Indicates the PR's author has not signed the CLA. labels Nov 7, 2018

knative-prow-robot merged commit 9617713 into knative:master Nov 7, 2018

neosab mentioned this pull request Nov 8, 2018

Adds Kafka Channel Provisioner Controllers #468

Merged

Harwayne mentioned this pull request Feb 7, 2019

Add Harwayne as an approver. #782

Merged

creydr added a commit to creydr/knative-eventing that referenced this pull request Mar 18, 2024

Add owning-component annotation to config-openshift-trusted-cabundle …

2245823

…ConfigMap (knative#560)

Conversation

neosab commented Oct 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neosab commented Oct 24, 2018

Uh oh!

adamharwayne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

neosab commented Oct 24, 2018

Uh oh!

adamharwayne commented Oct 29, 2018

Uh oh!

neosab commented Oct 29, 2018

Uh oh!

neosab commented Oct 29, 2018

Uh oh!

adamharwayne left a comment

Choose a reason for hiding this comment

Uh oh!

adamharwayne Oct 30, 2018

Choose a reason for hiding this comment

Uh oh!

n3wscott Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

neosab Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

adamharwayne Oct 30, 2018

Choose a reason for hiding this comment

Uh oh!

neosab Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

adamharwayne Oct 30, 2018

Choose a reason for hiding this comment

Uh oh!

n3wscott commented Oct 31, 2018

Uh oh!

googlebot commented Nov 7, 2018

Uh oh!

knative-metrics-robot commented Nov 7, 2018

Uh oh!

matzew commented Nov 7, 2018

Uh oh!

matzew commented Nov 7, 2018

Uh oh!

vaikas commented Nov 7, 2018

Uh oh!

knative-prow-robot commented Nov 7, 2018

Uh oh!

neosab commented Nov 7, 2018

Uh oh!

matzew commented Nov 7, 2018

Uh oh!

googlebot commented Nov 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

neosab commented Oct 24, 2018 •

edited

Loading