Skip to content

apis: flesh out KubernetesApplySpec#4627

Merged
nicks merged 1 commit into
masterfrom
nicks/k8s3
Jun 10, 2021
Merged

apis: flesh out KubernetesApplySpec#4627
nicks merged 1 commit into
masterfrom
nicks/k8s3

Conversation

@nicks

@nicks nicks commented Jun 9, 2021

Copy link
Copy Markdown
Contributor

Hello @milas, @landism,

Please review the following commits I made in branch nicks/k8s3:

6937ee6 (2021-06-09 15:53:18 -0400)
apis: flesh out KubernetesApplySpec

118d37c (2021-06-09 12:45:15 -0400)
apis: add KubernetesApply, ImageMap types

Code review reminders, by giving a LGTM you attest that:

  • Commits are adequately tested
  • Code is easy to understand and conforms to style guides
  • Incomplete code is marked with TODOs
  • Code is suitably instrumented with logging and metrics

@nicks
nicks requested review from landism and milas June 9, 2021 20:02
@nicks
nicks changed the base branch from master to nicks/k8s June 9, 2021 20:02
@nicks

nicks commented Jun 9, 2021

Copy link
Copy Markdown
Contributor Author

similarly, this is mostly fields and comments; everything else is autogenerated

Base automatically changed from nicks/k8s to master June 9, 2021 22:27
@nicks
nicks force-pushed the nicks/k8s3 branch 2 times, most recently from dbc136a to b963244 Compare June 10, 2021 15:27
package v1alpha1

// Selector for any Kubernetes-style API.
type ObjectSelector struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not critical now because it happens later anyway, but it might be worth making this implement Validator and ensure the regexes compile so you get an error upfront when trying to create/edit the entity. (I'm 80% sure that the codegen recursively calls validate functions if subfields implement Validator)

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.

hmmmm...ya, let me leave a TODO for this...I not totally sure when/how the validation gets called, and would want to be able to test it better once we're using the apiserver directly

@nicks
nicks merged commit 4d6729c into master Jun 10, 2021
@nicks
nicks deleted the nicks/k8s3 branch June 10, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants