Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Document all feed controller methods #305

Merged
merged 2 commits into from
Aug 3, 2018
Merged

Document all feed controller methods #305

merged 2 commits into from
Aug 3, 2018

Conversation

grantr
Copy link
Contributor

@grantr grantr commented Aug 1, 2018

updateFinalizers was deleted as it was unused.

updateOwnerReferences is still used, but that usage will be removed in an upcoming PR so it's not worth documenting here.

/cc @n3wscott

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

grantr commented Aug 1, 2018

/retest

1 similar comment
@grantr
Copy link
Contributor Author

grantr commented Aug 1, 2018

/retest

@adrcunha
Copy link
Contributor

adrcunha commented Aug 1, 2018

/test pull-knative-eventing-integration-tests

1 similar comment
@adrcunha
Copy link
Contributor

adrcunha commented Aug 1, 2018

/test pull-knative-eventing-integration-tests

@evankanderson
Copy link
Member

/assign @n3wscott

For initial review.

/assign @evankanderson

To approve after LGTM.

@grantr
Copy link
Contributor Author

grantr commented Aug 1, 2018

/retest

1 similar comment
@grantr
Copy link
Contributor Author

grantr commented Aug 2, 2018

/retest

@n3wscott
Copy link
Contributor

n3wscott commented Aug 2, 2018

/test pull-knative-eventing-integration-tests

@grantr
Document all feed controller methods
208f7d1 
updateFinalizers was deleted as it was unused.
@n3wscott
Copy link
Contributor

n3wscott commented Aug 3, 2018

/lgtm

This is a nice improvement! Thanks!

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2018
@google-prow-robot google-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2018
@knative-metrics-robot
Copy link

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

File Old Coverage New Coverage Delta
pkg/controller/feed/reconcile.go 69.4% 71.7% 2.3

@grantr
Copy link
Contributor Author

grantr commented Aug 3, 2018

Conflicts resolved. @evankanderson ready for /approve 😄

@n3wscott
Copy link
Contributor

n3wscott commented Aug 3, 2018

/lgtm

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2018
evankanderson
evankanderson reviewed Aug 3, 2018
View reviewed changes
Copy link
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

My VSCode will like this, thanks!

@google-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@google-prow-robot google-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2018
@google-prow-robot google-prow-robot merged commit d53a612 into knative:master Aug 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evankanderson evankanderson evankanderson left review comments

@n3wscott n3wscott Awaiting requested review from n3wscott

Assignees

@evankanderson evankanderson

@n3wscott n3wscott

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.
6 participants
@grantr @adrcunha @evankanderson @n3wscott @knative-metrics-robot @google-prow-robot