Skip to content

Adding standard isReady method to Channel.#515

Closed
n3wscott wants to merge 1 commit into
knative:masterfrom
n3wscott:channel-is-ready
Closed

Adding standard isReady method to Channel.#515
n3wscott wants to merge 1 commit into
knative:masterfrom
n3wscott:channel-is-ready

Conversation

@n3wscott
Copy link
Copy Markdown
Contributor

@n3wscott n3wscott commented Oct 10, 2018

Proposed Changes

  • All other eventing objects have a IsReady helper except Channel, adding that and InitializeConditions

Release Note

NONE

@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Oct 10, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: n3wscott
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: evankanderson

If they are not already assigned, you can assign the PR to them by writing /assign @evankanderson in a comment when ready.

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 size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 10, 2018
@knative-metrics-robot
Copy link
Copy Markdown

knative-metrics-robot commented Oct 10, 2018

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/apis/eventing/v1alpha1/channel_types.go 100.0% 33.3% -66.7

neosab
neosab reviewed Oct 10, 2018
View reviewed changes
Comment thread pkg/apis/eventing/v1alpha1/channel_types.go
}

// InitializeConditions sets relevant unset conditions to Unknown state.
func (cs *ChannelStatus) InitializeConditions() {
Copy link
Copy Markdown
Contributor

@neosab neosab Oct 10, 2018

Choose a reason for hiding this comment

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

Just FYI, I added InitializeConditions in my PR #468 along with a test.

Copy link
Copy Markdown
Contributor Author

@n3wscott n3wscott Oct 10, 2018

Choose a reason for hiding this comment

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

Oh nice!! can you also add isReady to #468 and I will close this...

Copy link
Copy Markdown
Contributor

@neosab neosab Oct 10, 2018

Choose a reason for hiding this comment

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

Done! 4c8232d

@n3wscott n3wscott mentioned this pull request Oct 11, 2018
Merged
@n3wscott
Copy link
Copy Markdown
Contributor Author

n3wscott commented Oct 11, 2018

/hold

I am asking #468 to be split into another PR with just the methods and tests added and if that happens it will supersede this PR.

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 11, 2018
@neosab neosab mentioned this pull request Oct 11, 2018
Merged
@n3wscott
Copy link
Copy Markdown
Contributor Author

n3wscott commented Oct 11, 2018

winner: #518

@n3wscott n3wscott closed this Oct 11, 2018
matzew pushed a commit to matzew/eventing that referenced this pull request Apr 8, 2020
@vdemeester
Removing myself from owners/reviewers (knative#515)
f86cca2
pierDipi pushed a commit to pierDipi/eventing that referenced this pull request Feb 7, 2024
@Cali0707
Don't use async handler (knative#7415) (knative#515)
2303bba 
* Don't use async handler



* Fix Kn-Namespace header, potential race condition for sync receiver



* try refactoring test



* take 2 on test fix



---------

Signed-off-by: Calum Murray <cmurray@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scothis scothis Awaiting requested review from scothis

@vaikas vaikas Awaiting requested review from vaikas

1 more reviewer

@neosab neosab neosab left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@n3wscott @knative-prow-robot @knative-metrics-robot @neosab