Skip to content

Nothing to validate with event sources. Removing todos.#400

Merged
knative-prow-robot merged 2 commits into
knative:masterfrom
n3wscott:webhooks_eventsource
Aug 29, 2018
Merged

Nothing to validate with event sources. Removing todos.#400
knative-prow-robot merged 2 commits into
knative:masterfrom
n3wscott:webhooks_eventsource

Conversation

@n3wscott
Copy link
Copy Markdown
Contributor

@n3wscott n3wscott commented Aug 28, 2018

Follow-up to: #352

Proposed Changes

  • Remove TODOs for [Cluster]EventSource validation, nothing to do.

Release Note

NONE

@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 28, 2018
@n3wscott
Copy link
Copy Markdown
Contributor Author

n3wscott commented Aug 28, 2018

/assign @vaikas-google

@adamharwayne
Copy link
Copy Markdown
Contributor

adamharwayne commented Aug 28, 2018

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2018
vaikas
vaikas reviewed Aug 29, 2018
View reviewed changes
Comment thread pkg/apis/feeds/v1alpha1/cluster_event_source_validation.go Outdated
}

func (cess *ClusterEventSourceSpec) Validate() *apis.FieldError {
// nothing to validate
Copy link
Copy Markdown
Contributor

@vaikas vaikas Aug 29, 2018

Choose a reason for hiding this comment

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

nit: I find this comment odd, since we then do call validate on the CESS.

Copy link
Copy Markdown
Contributor Author

@n3wscott n3wscott Aug 29, 2018

Choose a reason for hiding this comment

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

I mean noting to validate for fields that are managed directly by ClusterEventSourceSpec. I suppose I can just remove the comment and continue cess.CommonEventSourceSpec.Validate()

Comment thread pkg/apis/feeds/v1alpha1/event_source_validation.go Outdated
}

func (ess *EventSourceSpec) Validate() *apis.FieldError {
// nothing to validate
Copy link
Copy Markdown
Contributor

@vaikas vaikas Aug 29, 2018

Choose a reason for hiding this comment

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

ditto here.

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

n3wscott commented Aug 29, 2018

Updated based on @vaikas-google 's feedback

@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Aug 29, 2018

/lgtm
/approve

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

knative-prow-robot commented Aug 29, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, 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:

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 29, 2018
@knative-prow-robot knative-prow-robot merged commit 5533ffd into knative:master Aug 29, 2018
matzew added a commit to matzew/eventing that referenced this pull request Jan 27, 2020
@matzew
Merge pull request knative#400 from matzew/use_ci_CS
82e2abd 
using the CI catalog source ..
pierDipi added a commit to pierDipi/eventing that referenced this pull request Nov 17, 2023
@pierDipi
Add encryption and auth tests (knative#400)
ade9d79 
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grantr grantr Awaiting requested review from grantr

@mattmoor mattmoor Awaiting requested review from mattmoor

1 more reviewer

@vaikas vaikas vaikas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@adamharwayne adamharwayne

@vaikas vaikas

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/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@n3wscott @adamharwayne @vaikas @knative-prow-robot