-
Notifications
You must be signed in to change notification settings - Fork 584
infra: Add more constants for platforms and update godoc #182
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.
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
infra: Add more constants for platforms and update godoc #182
Conversation
Expand the list of constants, clarify that unsupported platforms must be handled as None, and improve godoc. These are not supported platforms, these are the constants that will be returned.
|
As part of openshift/installer#1112 clarify the set of constants and document the "unrecognized behavior must be None". This ensures we don't use inconsistent constants (installer and mco don't match these values yet, but will need to before API freeze) @abhinavdahiya @crawford @bparees @ironcladlou @ashcrow as folks who have components that will need to ensure toleration exists. |
| // "OpenStack", "VSphere", and "None". Individual components may not support | ||
| // all platforms, and must handle unrecognized platforms as None if they do | ||
| // not support that platform. | ||
| Platform PlatformType `json:"platform,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // other integrations are enabled. If None, no infrastructure automation is | ||
| // enabled. | ||
| // enabled. Allowed values are "AWS", "Azure", "GCP", "Libvirt", | ||
| // "OpenStack", "VSphere", and "None". Individual components may not support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need advice for components to indicate their lack of support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do, but that'll be in #1112. I don't want our public api docs to be instructions to our teams (beyond general statements), especially since we don't know the instructions quite yet.
|
Any other comments? |
|
Assuming we get our directions for how to indicate that we didn't fulfill intent /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, smarterclayton 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 |
|
this looks generally fine to me as well. agree w/ david's comment. |
Expand the list of constants, clarify that unsupported platforms
must be handled as None, and improve godoc. These are not supported
platforms, these are the constants that will be returned.