Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Tests for filter.go and comments in types.go #2055

Merged
merged 9 commits into from
May 23, 2018
Merged

Tests for filter.go and comments in types.go #2055

merged 9 commits into from
May 23, 2018

Conversation

teaguecole
Copy link
Contributor

I accidentally closed my PR... long story, But here it is again with a fix for scotts comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 21, 2018
@@ -1314,9 +1316,9 @@ const (
// SpecServiceBrokerName is used for ServiceClasses, the parent service broker name.
FilterSpecServiceBrokerName = "spec.serviceBrokerName"
// SpecClusterServiceClassName is only used for plans, the parent service class name.
FilterSpecClusterServiceClassName = "spec.clusterServiceClass.name"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please undo this change

// name - the value set to [Cluster]ServicePlan.Name
// spec.externalName - the value set to [Cluster]ServicePlan.Spec.ExternalName
// spec.externalID - the value set to [Cluster]ServicePlan.Spec.ExternalID
// spec.serviceClassName - the value set to [Cluster]ServicePlan.Spec.ServiceClassRef.Name
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this line to:

//   spec.serviceClass.name - the value set to ServicePlan.Spec.ServiceClassRef.Name
//   spec.clusterServiceClass.name - the value set to ClusterServicePlan.Spec.ClusterServiceClassRef.Name

// SpecServiceClassName is only used for plans, the parent service class name.
FilterSpecServiceClassName = "spec.serviceClass.name"
Copy link
Contributor

Choose a reason for hiding this comment

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

And undo this change.

// name - the value set to [Cluster]ServicePlan.Name
// spec.externalName - the value set to [Cluster]ServicePlan.Spec.ExternalName
// spec.externalID - the value set to [Cluster]ServicePlan.Spec.ExternalID
// spec.serviceClassName - the value set to [Cluster]ServicePlan.Spec.ServiceClassRef.Name
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments apply to here from the other types file

@teaguecole
Copy link
Contributor Author

@carolynvs put on those kicking boots please, this build needs a kick. (Relates to #2036)

@carolynvs
Copy link
Contributor

Kicked!

Copy link
Contributor

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for resubmitting and sorry for all the trouble!

@n3wscott n3wscott added the LGTM2 label May 23, 2018
@MHBauer MHBauer merged commit a361d83 into kubernetes-retired:master May 23, 2018
@teaguecole teaguecole deleted the salvage/1986/PR2021 branch May 24, 2018 01:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 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