Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Adding Topic Controller #35

Merged
merged 24 commits into from
Jul 8, 2019
Merged

Adding Topic Controller #35

merged 24 commits into from
Jul 8, 2019

Conversation

n3wscott
Copy link
Contributor

@n3wscott n3wscott commented Jun 19, 2019 •
edited
Loading

fixes #50

@n3wscott n3wscott changed the title [WIP] Adding topic rec [WIP] Adding Topic Controller Jun 19, 2019
@n3wscott
Copy link
Contributor Author

dep on #34

@n3wscott n3wscott changed the title [WIP] Adding Topic Controller Adding Topic Controller Jun 20, 2019
@n3wscott
Copy link
Contributor Author

/test pull-googlecloudplatform-cloud-run-events-go-coverage

@n3wscott n3wscott mentioned this pull request Jun 24, 2019
Closed
grantr
grantr reviewed Jun 27, 2019
View reviewed changes
Copy link
Contributor

@grantr grantr left a comment

Choose a reason for hiding this comment

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

Not all the way through yet, but here's what I've got so far

Gopkg.toml Show resolved Hide resolved
Gopkg.toml Show resolved Hide resolved
cmd/pubsub/publisher/main.go Outdated Show resolved Hide resolved
cmd/pubsub/publisher/main.go Show resolved Hide resolved
cmd/pubsub/publisher/main.go Show resolved Hide resolved
cmd/pubsub/publisher/main.go Outdated Show resolved Hide resolved
pkg/pubsub/publisher/publisher.go Show resolved Hide resolved
grantr
grantr reviewed Jun 27, 2019
View reviewed changes
Copy link
Contributor

@grantr grantr left a comment

Choose a reason for hiding this comment

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

Got a bit further. Bookmarking https://github.com/GoogleCloudPlatform/cloud-run-events/pull/35/files#diff-1d699b9cd7b3d4b5ad8802c07777a902R138

pkg/reconciler/testing/topic.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/controller.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/controller.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/resources/labels.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/resources/publisher.go Show resolved Hide resolved
pkg/reconciler/topic/resources/publisher.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
@n3wscott n3wscott mentioned this pull request Jun 28, 2019
Closed
grantr
grantr reviewed Jun 28, 2019
View reviewed changes
Copy link
Contributor

@grantr grantr left a comment

Choose a reason for hiding this comment

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

Bookmark https://github.com/GoogleCloudPlatform/cloud-run-events/pull/35/files#diff-1d699b9cd7b3d4b5ad8802c07777a902R253

pkg/reconciler/topic/topic.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/topic.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Outdated Show resolved Hide resolved
pkg/reconciler/topic/topic.go Outdated Show resolved Hide resolved
grantr
grantr reviewed Jun 28, 2019
View reviewed changes
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic.go Show resolved Hide resolved
pkg/reconciler/topic/topic_test.go Show resolved Hide resolved
This was referenced Jun 28, 2019
Closed
Closed
@grantr grantr mentioned this pull request Jun 28, 2019
Closed
@n3wscott
Copy link
Contributor Author

Let's follow back up on Project vs ProjectID. I prefer just Project; but there are samples that use either depending on author.

Merge this so we can make channels.

@knative-metrics-robot
Copy link

The following is the coverage report on pkg/.
Say /test pull-googlecloudplatform-cloud-run-events-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pubsub/v1alpha1/topic_lifecycle.go 69.2% 78.3% 9.0
pkg/reconciler/topic/controller.go Do not exist 93.3%
pkg/reconciler/topic/resources/labels.go Do not exist 100.0%
pkg/reconciler/topic/resources/publisher.go Do not exist 71.4%
pkg/reconciler/topic/topic.go Do not exist 67.3%

@grantr grantr mentioned this pull request Jul 8, 2019
Closed
This was referenced Jul 8, 2019
Closed
Closed
@grantr grantr mentioned this pull request Jul 8, 2019
Closed
grantr
grantr approved these changes Jul 8, 2019
View reviewed changes
Copy link
Contributor

@grantr grantr left a comment

Choose a reason for hiding this comment

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

/lgtm

All my comments are either addressed or broken out into followup issues. Thanks for bearing with me on this huge review!

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grantr, n3wscott

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

@knative-prow-robot knative-prow-robot merged commit b1be21c into google:master Jul 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@evankanderson evankanderson evankanderson left review comments

@jonjohnsonjr jonjohnsonjr jonjohnsonjr left review comments

@grantr grantr grantr approved these changes

@Harwayne Harwayne Awaiting requested review from Harwayne

@vaikas vaikas Awaiting requested review from vaikas

Assignees

@grantr grantr

Labels
approved lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pubsub Publisher should be a ksvc.
6 participants
@n3wscott @knative-metrics-robot @knative-prow-robot @grantr @evankanderson @jonjohnsonjr