Refactor the Service following #2447#2500
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mattmoor The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
left a comment
There was a problem hiding this comment.
@mattmoor: 11 warnings.
Details
In response to this:
This starts to factor out functional options for setting up our table tests, which reduces the verbosity a fair amount.
I've broken off Service from a broader set of changes in #2447 to make things a bit more manageable to review.
WIP until #2493 lands
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
| } | ||
| ) | ||
|
|
||
| func WithRunLatestRollout(s *v1alpha1.Service) { |
There was a problem hiding this comment.
Golint comments: exported function WithRunLatestRollout should have comment or be unexported. More info.
| } | ||
| } | ||
|
|
||
| func WithFailedConfig(name, reason, message string) ServiceOption { |
There was a problem hiding this comment.
Golint comments: exported function WithFailedConfig should have comment or be unexported. More info.
| } | ||
| } | ||
|
|
||
| func WithPinnedRollout(name string) ServiceOption { |
There was a problem hiding this comment.
Golint comments: exported function WithPinnedRollout should have comment or be unexported. More info.
| } | ||
| } | ||
|
|
||
| func WithReadyConfig(name string) ServiceOption { |
There was a problem hiding this comment.
Golint comments: exported function WithReadyConfig should have comment or be unexported. More info.
| }) | ||
| } | ||
|
|
||
| func WithFailedRoute(reason, message string) ServiceOption { |
There was a problem hiding this comment.
Golint comments: exported function WithFailedRoute should have comment or be unexported. More info.
| } | ||
| } | ||
|
|
||
| func WithManualRollout(s *v1alpha1.Service) { |
There was a problem hiding this comment.
Golint comments: exported function WithManualRollout should have comment or be unexported. More info.
| s.Status.SetManualStatus() | ||
| } | ||
|
|
||
| func WithReadyRoute(s *v1alpha1.Service) { |
There was a problem hiding this comment.
Golint comments: exported function WithReadyRoute should have comment or be unexported. More info.
| } | ||
| } | ||
|
|
||
| // TODO(mattmoor): Ideally this could be a more generic Option and use meta.Accessor, |
There was a problem hiding this comment.
Golint comments: comment on exported function WithCreationTimestamp should be of the form "WithCreationTimestamp ...". More info.
| } | ||
| } | ||
|
|
||
| func WithInitSvcConditions(s *v1alpha1.Service) { |
There was a problem hiding this comment.
Golint comments: exported function WithInitSvcConditions should have comment or be unexported. More info.
| s.Status.InitializeConditions() | ||
| } | ||
|
|
||
| func WithManualStatus(s *v1alpha1.Service) { |
There was a problem hiding this comment.
Golint comments: exported function WithManualStatus should have comment or be unexported. More info.
4b3caf6 to
29f6b1d
Compare
|
/lint |
knative-prow-robot
left a comment
There was a problem hiding this comment.
@mattmoor: 1 unresolved warnings and 1 new warnings.
Details
In response to this:
/lint
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
| } | ||
| } | ||
|
|
||
| // WithFailedCondif reflects the Configuration's failure in the Service |
There was a problem hiding this comment.
Golint comments: comment on exported function WithFailedConfig should be of the form "WithFailedConfig ...". More info.
29f6b1d to
5a61135
Compare
|
/lint |
knative-prow-robot
left a comment
There was a problem hiding this comment.
@mattmoor: 1 unresolved warning and 0 new warning.
Details
In response to this:
/lint
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
This starts to factor out functional options for setting up our table tests, which reduces the verbosity a fair amount. I've broken off Service from a broader set of changes in knative#2447 to make things a bit more manageable to review.
5a61135 to
4139e4b
Compare
|
/lint |
knative-prow-robot
left a comment
There was a problem hiding this comment.
@mattmoor: 0 warnings.
Details
In response to this:
/lint
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
| Object: svc("all-ready", "foo", WithRunLatestRollout, | ||
| // When the Route and Configuration come back with ready | ||
| // our initial conditions transition to Ready=True. | ||
| // TODO(mattmoor): Add Latest{Created,REady} |
|
This was merged via the change stacked on top of it. |
This starts to factor out functional options for setting up our table tests, which reduces the verbosity a fair amount.
I've broken off Service from a broader set of changes in #2447 to make things a bit more manageable to review.
WIP until #2493 lands