-
Notifications
You must be signed in to change notification settings - Fork 7
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
OPSEXP-2242: remove usage of lookup in alfresco-repository #105
Conversation
@@ -233,3 +239,7 @@ global: | |||
alfrescoRegistryPullSecrets: quay-registry-secret | |||
# -- a fallback for .Values.known_urls that can be shared between charts | |||
known_urls: | |||
|
|||
# @ignore |
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.
it is not better to document the purpose of this? users are going to discover it anyway by looking at values
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.
I think it would just be "doc pollution" as no one is ever supposed to use that. If we end up introducing something valuable to users as tags then we would document it
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.
users are not supposed to use that but I see a value telling them explicitly "this is just for testing the chart - don't use it"
keys: | ||
# -- Key within the secret holding the message broker username | ||
username: BROKER_USERNAME | ||
# -- Key within the secret holding the message broker password | ||
password: BROKER_PASSWORD | ||
existingConfigMap: | ||
# -- Name of a pre-existing configmap containing the meesage broker URL | ||
name: null | ||
name: "" |
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.
why?
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.
This is needed by the tpl()
function which expects a string and breaks in case of a null object instead. But as I write this I think it would be better to cast the value when using the function to make the whole thing more robust... wdyt?
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.
well if it's just for that it's better to handle that specific case there and do not pollute values with empty strings instead of plain nulls
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.
reverted the whole tpl thing for now
Ref: OPSEXP-2242
Reverting as unusable when called from a parent chart