-
Notifications
You must be signed in to change notification settings - Fork 26
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
SetHostnameAsFQDN option is added to all components. Default is true #302
Conversation
if diff := cmp.Compare(slices.Index(statusOrder, a.Status), slices.Index(statusOrder, b.Status)); diff != 0 { | ||
return diff | ||
} | ||
return a.LastTransitionTime.Compare(b.LastTransitionTime.Time) |
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.
Our linter test didn't catch that last time? =/
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.
Looks like not
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.
shouln't diff be gone after 6a6f343 ?
Maybe need to rebase
it seems the added options isn't used in podtemplate generation. it still has the hardcoded true value there |
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.
Agree with @kruftik, it looks that it must be passed deeper.
And also, maybe we can set this default in one place in the server, not every component?
Or use default in spec
I suggest we use spec default true for simplicity and check for the diff when will update ytop. If there is a diff — will mention in the release notes |
lets just use the value from the spec and set |
…stanceSpec in one place in server.go
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.
LGTM if default works and we not making field = false for users.
Fix #295 |
…tsaurus#302) * SetHostnameAsFQDN option is added to all components. Default is true * SetHostnameAsFQDN is true by default and the value is getting from instanceSpec in one place in server.go * forgot lint-generated
No description provided.