-
Notifications
You must be signed in to change notification settings - Fork 21
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
util: Remove BuildGitInfo #1109
Conversation
We already have other ways of determining the version at runtime (namely: the tag associated with the images), and this just adds needless complication to our dockerfiles.
// See also: | ||
// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set | ||
LabelPluginCreatedMigration: gitVersion, | ||
LabelPluginCreatedMigration: "true", |
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.
is it okay that the format of the label value has changed?
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.
Yup -- double-checked that the only place we use this label is here:
autoscaling/pkg/plugin/watch.go
Lines 362 to 369 in 4156e39
metav1.ListOptions{ | |
// NB: Including just the label itself means that we select for objects that *have* the | |
// label, without caring about the actual value. | |
// | |
// See also: | |
// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#set-based-requirement | |
LabelSelector: LabelPluginCreatedMigration, | |
}, |
... where we don't require that it's set to any particular value :)
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 good, dropped one minor question about label format
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. HTML Report |
Follow-up to #1109, must have forgotten it there. These are no longer used.
Follow-up to #1109, must have forgotten it there. These are no longer used.
We already have other ways of determining the version at runtime (namely: the tag associated with the images), and this just adds needless complication to our dockerfiles.