Skip to content

Test ParametersFrom feature#307

Merged
google-prow-robot merged 2 commits into
knative:masterfrom
grantr:clean-up-methods
Aug 3, 2018
Merged

Test ParametersFrom feature#307
google-prow-robot merged 2 commits into
knative:masterfrom
grantr:clean-up-methods

Conversation

@grantr
Copy link
Copy Markdown
Contributor

@grantr grantr commented Aug 1, 2018

Adds a test exercising the ParametersFrom secret key reference. Also removes a superfluous helper method that was only used in one place and didn't make that place more terse or more clear.

grantr added 2 commits August 1, 2018 15:24
It's only used in one place, and that usage doesn't save any lines of
code.
Adds a test of a feed with a ParametersFrom secret key reference.
@google-prow-robot google-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 1, 2018
@knative-metrics-robot
Copy link
Copy Markdown

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 60.4% 68.3% 8.0

return feed
}

func getSecretStartInProgressFeed() *feedsv1alpha1.Feed {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dprotaso The builder code from knative/serving#1762 would have been helpful here.

@grantr
Copy link
Copy Markdown
Contributor Author

grantr commented Aug 1, 2018

/retest

Copy link
Copy Markdown
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

Generally, I'd discourage touching test code & code under test at the same time, but this seems like a straightforward cleanup.

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

[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

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

@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 1, 2018
@grantr
Copy link
Copy Markdown
Contributor Author

grantr commented Aug 2, 2018

/retest

@n3wscott
Copy link
Copy Markdown
Contributor

n3wscott commented Aug 3, 2018

/test pull-knative-eventing-integration-tests

@google-prow-robot google-prow-robot merged commit 4f80f18 into knative:master Aug 3, 2018
@grantr grantr deleted the clean-up-methods branch August 3, 2018 17:05
n3wscott pushed a commit to n3wscott/eventing that referenced this pull request Aug 3, 2018
* Remove unmarshalJSON method

It's only used in one place, and that usage doesn't save any lines of
code.

* Test the secret retrieval code

Adds a test of a feed with a ParametersFrom secret key reference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

5 participants