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

using cloud events for the sample heartbeat #27

Merged
merged 8 commits into from
Oct 30, 2018
Merged

using cloud events for the sample heartbeat #27

merged 8 commits into from
Oct 30, 2018

Conversation

n3wscott
Copy link
Contributor

Proposed Changes

  • Use CloudEvents from knative/eventing for the heartbeat sample.

Release Note

NONE

/assign @grantr

@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 24, 2018
matzew
matzew reviewed Oct 25, 2018
View reviewed changes
cmd/event_receiver/main.go Outdated Show resolved Hide resolved
matzew
matzew reviewed Oct 25, 2018
View reviewed changes
cmd/heartbeats/main.go Outdated Show resolved Hide resolved
@n3wscott
Copy link
Contributor Author

Thanks for the conflicts @vaikas-google :D

grantr
grantr approved these changes Oct 29, 2018
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
/hold

Looks better now, thanks @n3wscott. I have a few optional suggestions, remove hold when ready.

Seems like a few unrelated changes made it into this PR (a change to hack/update-manifests.sh, CRD labels, RBAC on ContainerSource controller) but those will probably be fixed when you fix merge conflicts.

Gopkg.lock
@@ -854,6 +862,8 @@
"github.com/emicklei/go-restful",
"github.com/google/go-cmp/cmp",
"github.com/google/go-cmp/cmp/cmpopts",
"github.com/google/uuid",
"github.com/knative/eventing/pkg/event",
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this package continue to exist in knative after knative/eventing#532 is merged? If we intend to share it, maybe it should move to knative/pkg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cross that bridge when we get there...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#45

cmd/event_receiver/main.go Outdated Show resolved Hide resolved
cmd/heartbeats/main.go Show resolved Hide resolved
cmd/heartbeats/main.go Outdated Show resolved Hide resolved
cmd/heartbeats/main.go Show resolved Hide resolved
@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 29, 2018
@n3wscott n3wscott mentioned this pull request Oct 29, 2018
Closed
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 29, 2018
@n3wscott
Copy link
Contributor Author

PTAL @grantr

grantr
grantr approved these changes Oct 29, 2018
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

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2018
@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 removed the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2018
@grantr
Copy link
Contributor

grantr commented Oct 29, 2018

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2018
@n3wscott
Copy link
Contributor Author

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 30, 2018
@knative-prow-robot knative-prow-robot merged commit 359ea93 into knative:master Oct 30, 2018
@matzew matzew mentioned this pull request Sep 10, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matzew matzew matzew left review comments

@grantr grantr grantr approved these changes

@evankanderson evankanderson Awaiting requested review from evankanderson

Assignees

@grantr grantr

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@n3wscott @knative-prow-robot @grantr @matzew