Skip to content

Make ProvisioningNetwork values an enum#19

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
sadasu:provisioning-network-enum
Sep 23, 2020
Merged

Make ProvisioningNetwork values an enum#19
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
sadasu:provisioning-network-enum

Conversation

@sadasu
Copy link
Contributor

@sadasu sadasu commented Sep 22, 2020

No description provided.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 22, 2020
type ProvisioningNetwork string

const (
provisioningNetworkManaged = "Managed"
Copy link
Contributor

Choose a reason for hiding this comment

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

These consts can be defined as being of the new type. I think if you do that, you don't need line 25.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed these constants from the latest changes because they are not necessary to generate the right yaml file. Will add them as part of the validation code that is in review.

- Managed
- Unmanaged
- Disabled
type: string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making the change in #19 (comment) made these changes in 46 - 49 disappear.

// the external network that would be used for provisioning services.
ProvisioningNetwork string `json:"provisioningNetwork,omitempty"`
ProvisioningNetwork ProvisioningNetwork `json:"provisioningNetwork,omitempty"`
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here the type of ProvisioningNetwork has changed but in the resulting yaml file it is still a string.

@sadasu sadasu force-pushed the provisioning-network-enum branch from d1b0747 to b1647cf Compare September 22, 2020 21:13
@sadasu sadasu force-pushed the provisioning-network-enum branch from b1647cf to 640297f Compare September 22, 2020 21:32
@sadasu
Copy link
Contributor Author

sadasu commented Sep 22, 2020

@sadasu
Copy link
Contributor Author

sadasu commented Sep 22, 2020

/cc @asalkeld

@sadasu
Copy link
Contributor Author

sadasu commented Sep 22, 2020

/retest

// +kubebuilder:object:root=true
// +kubebuilder:resource:path=provisionings,scope=Cluster
// +kubebuilder:subresource:status

Copy link
Contributor

Choose a reason for hiding this comment

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

any reason for moving these back up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: asalkeld, sadasu

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

@openshift-merge-robot openshift-merge-robot merged commit e4c2a38 into openshift:master Sep 23, 2020
@sadasu sadasu deleted the provisioning-network-enum branch January 7, 2021 13:45
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants