Skip to content

k8s: handle CRDs properly#953

Merged
nicks merged 1 commit into
masterfrom
nicks/ch1236/crd
Jan 9, 2019
Merged

k8s: handle CRDs properly#953
nicks merged 1 commit into
masterfrom
nicks/ch1236/crd

Conversation

@nicks

@nicks nicks commented Jan 8, 2019

Copy link
Copy Markdown
Contributor

Hello @landism, @maiamcc,

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

9dd876e (2019-01-08 17:27:42 -0500)
k8s: handle CRDs properly

@nicks
nicks requested review from landism and maiamcc January 8, 2019 22:28
@nicks
nicks force-pushed the nicks/ch1236/crd branch from 9dd876e to 23067ed Compare January 8, 2019 22:30

@maiamcc maiamcc left a comment

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.

lgtm -- tho also i think i lack context. what problem does this fix?

Comment thread internal/k8s/serialize_test.go Outdated

func TestCRDYAML(t *testing.T) {
entities, err := ParseYAMLFromString(testyaml.CRDYAML)
assert.NoError(t, err)

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.

you should if err != nil {t.Fatal(err)} here so we don't continue to execute even if this step is borked and then get weird and confusing errors further down the test.

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.

i added a helper for it!

@nicks

nicks commented Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

did you see the test case? there is a good example of a YAML config that didn't work before, and works now

@nicks
nicks force-pushed the nicks/ch1236/crd branch from 23067ed to 0291a32 Compare January 9, 2019 17:16
@nicks
nicks force-pushed the nicks/ch1236/crd branch from 0291a32 to 1d6b200 Compare January 9, 2019 17:33
@nicks
nicks merged commit 73e6836 into master Jan 9, 2019
@nicks
nicks deleted the nicks/ch1236/crd branch January 9, 2019 17:44
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