Skip to content
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

New schema for params operator file #1020

Merged
merged 7 commits into from
Nov 1, 2019
Merged

New schema for params operator file #1020

merged 7 commits into from
Nov 1, 2019

Conversation

kensipe
Copy link
Member

@kensipe kensipe commented Oct 31, 2019

What this PR does / why we need it:

Makes param files one yaml doc, which will allow for a schema to be applied.

Fixes #1011

Copy link
Member

@gerred gerred left a comment

Choose a reason for hiding this comment

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

looks great, one errand commented line of code to remove.

pkg/kudoctl/packages/package.go Outdated Show resolved Hide resolved
@alenkacz alenkacz changed the title Ken/params one doc New schema of params operator file Nov 1, 2019
@alenkacz alenkacz changed the title New schema of params operator file New schema for params operator file Nov 1, 2019
t.Errorf("readParametersFile() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if !reflect.DeepEqual(got, tt.want) {
assert.Equal(t, tt.want, got)

will also print a nicely formatted diff of two.

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := readParametersFile(tt.fileBytes)
if (err != nil) != tt.wantErr {
Copy link
Contributor

Choose a reason for hiding this comment

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

One-liner to replace four:

Suggested change
if (err != nil) != tt.wantErr {
assert.Equal(t, tt.wantErr, err != nil, "readParametersFile() error = %v, wantErr %v", err, tt.wantErr)

Copy link
Contributor

@zen-dog zen-dog left a comment

Choose a reason for hiding this comment

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

Lgtm, aside from a few proposed assert usages. I suggest, we update operators dev branch with the new changes before merging this PR (so that this one should have ✅ e2e-tests)

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

LGTM

@kensipe kensipe merged commit 061b1eb into master Nov 1, 2019
@kensipe kensipe deleted the ken/params-one-doc branch November 1, 2019 14:39
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.

Convert params to single document
4 participants